Add detection of container/video_codec/audio_codec compatibility for live file streaming or transcoding #384

bnkai · 2020-02-25T18:48:51Z

Fixes #354
As it is now stash only selects which files are "streamable" by using the video_codec information from ffprobe stored in DB. In some files while the codec is indeed supported by the browser/jwplayer the container isn't and thus instead of live transcoding the files we get the error message from #354

the container isn't stored in the database so we either need to store it or simply call ffprobe before we begin the streaming of a media file. For now i chose to go with the second option and it doesn't seem to add any measurable player startup delay
a combination of valid codec/container pairs is used in addition to the codec validation. mp4 and webm seem to be the "safe" choises for containers so i whitelisted those
ffprobe treats webm and matroska as the same which is problematic for firefox and probably a lot more browsers (chrome doesn't have a problem). I had to use a "magic_file" reader to determine the actual container (reads 4kb of the file, no measurable delay here either) for this case .

As it is now all the error cases from #354 get live transcoded ok.

TODO / TBD

Browser Detection ( EDIT not to be implemented , the user agent header is way too abused to make a consistent identification of the web browser )
We could detect which browser is used and adjust the valid combinations accordingly ( user agent header is available in routes_scene ,i left a TODO there ). For example chrome (and probably most chromium browsers ) suppports matroska as a container for h264 while firefox doesn't. Prior to this PR mkv and h264 played directly in chrome while after this fix (as it is now) the files will get live transcoded
A force matroska option was implemented instead
Transcode generation ( EDIT completed)
I haven't touched the file transcode code yet.If we adjust it over there then there should be a new profile that copys the video stream and doesn't reencode as the codec is supported and only the container isn't. Something like ffmpeg -i input h264_media_file.avi -c:v copy .... output.mp4
Transcoded files (mp4) are generated correctly when the codec/container is not supported. For h264 files the video stream is directly copied avoiding time and quality loss.
Stream copy profile ( EDIT not to be implemented , the main benefit would be h264 encoded files but the resulting mp4 file will not be fully seekable )
Same as above for files that the container is not supported but the codec is. Instead of live transcoding using the webm live stream profile is it possible to stream only copying the video stream? Most use cases would be h264 in mpegts,flv,avi that the video could be copied to mp4 ( no reduction in quality compared to transcoding and almost no cpu usage ) .
EDIT streaming is possible by copying the video stream but for h264 (mkv,flv,ts,avi) the resulting mp4 file won't be fully seekable (the duration keeps increasing till the file is fully loaded to the player). ~~Is this ok or should we transcode this cases also to vp9.webm ?~~
EDIT After some more testing the fragmented mp4 (for the h264 mkv,flv,ts,avi ) that is not fully seekable is not acceptable imo.
Despite transcoding and loosing a bit in quality the file seeking/seek to sprite of the webm encoded file makes a better user experience.
Add container as an extra field to the scenes table. ( EDIT completed)
Is a cleaner solution
~~For new users it should be fairly easy to implement, it's a bit more tricky for users with existing DBs and more complex to implement than the current solution.~~
A single scan is needed to populate the format field (container) for already existing files.
Audio codec (EDIT completed)
Is it worth the extra complexity / code to check for valid audio codec ? I didn't see any issues with files playing the video part and not the audio.

bnkai · 2020-03-02T10:39:06Z

Added container detection to generate transcode task.
For h264 files that are not supported (ts,flv,avi,mkv ...) the video part is copied instead of being reencoded.

WithoutPants

One minor change.

Need to consider the performance impact of running ffprobe when streaming scenes that don't require transcode.

We can probably go ahead with this version and evaluate the performance impact, and add the container field if we deem it too much of a problem.

pkg/api/routes_scene.go

bnkai · 2020-03-06T04:02:29Z

Container info added to DB.
Before testing make sure to backup the database file as db appversion is increased to 4.
After migrating to this version a scan is needed to populate the missing container to all media files.
This scan will take some time as it ffprobes all files to get the info.
Once the first scan is finished all the next ones are as fast as before.

WithoutPants · 2020-03-10T06:59:38Z

Can you please rebase against develop and update the migration to be 5? The schema version has been bumped on develop.

* take into account container in generate transcodes task * add container info to DB * increment appSchema to 5

WithoutPants · 2020-03-11T05:22:42Z

I'm seeing regression in behaviour for some combinations. It appears that previous combinations (tested with mp4/flv combination) that did not work for me now appear to work, but they do not allow seeking. That may be a limitation somewhere for this scenario. However, I noticed that on the current version in Chrome, the mp4/matroska combination works correctly with seeking, but in this PR, the seeking is disabled.

bnkai · 2020-03-11T08:26:59Z

Yes as i stated at the beginning i wanted the browser detection mainly for matroska and Chromium based browsers. What do you think about adding an extra conf option to "force" mkv as supported? Some files are non seekable anyway though and i don't think there is a way to fix that for live streaming ( only fully transcoding deals with that)

WithoutPants · 2020-03-11T08:41:58Z

What do you think about adding an extra conf option to "force" mkv as supported?

Yep, I'd like this.

The seeking stuff is because IsStreamable (in manager/utils.go) hasn't been updated with your new logic.

bnkai · 2020-03-11T20:35:30Z

Pushed a fix for manager/utils.go.
I switched to WIP till i implement the force mkv option.

edit adding as an extra fix for cases where live transcoding fails when ffmpeg doesn't support the audio codec ( as a fix transcode only the video part)

bnkai · 2020-03-13T18:01:06Z

Is ui/v2.5/src/core/generated-graphql.tsx supposed to be tracked by git ?
I didn't notice that it was generated and it got added to the commit....

…rted/unknown audio codecs

WithoutPants · 2020-03-13T21:09:06Z

Is ui/v2.5/src/core/generated-graphql.tsx supposed to be tracked by git ?

No. I just haven't yet bothered to add it to the gitignore file. Doesn't matter.

WithoutPants · 2020-03-17T01:52:34Z

When I set the force mkv to true, the mkv files are not transcoded as expected, but the seeking doesn't work - regardless of where you seek to, it starts from the start of the file.

bnkai · 2020-03-17T09:15:25Z

Does it print anything when you play those mkvs?
It shouldn't print anything(transcoding file ....) if they are served directly. Can you try the same files with the dev/master version?

WithoutPants · 2020-03-17T20:51:33Z

The same files stream directly and correctly with the dev version. There is no log entry indicating that the file is being transcoded, so I'm confident that it is not. When it does transcode (when the checkbox is cleared), the seeking works correctly.

bnkai · 2020-03-18T01:40:51Z

Can you try reloading the page with player / restarting stash after you save the configuration with the mkv as forced? The only thing i can think of is that you get a cached response for the isStreamable the player is requesting. I added a couple of prints that i'll remove later to check this out. Now you should get something printed when the player requests the isStreamable.

WithoutPants · 2020-03-19T01:35:37Z

You are correct. Refreshing the page fixes the issue. We may want to consider resetting the cache when setting those flags.

I also noticed that when I forced the mkv support, I didn't get sound on my particular video. The audio coded was ac3. I haven't yet checked if other ac3 videos work. Note that this happens with the current version as well, so not something introduced by your change. Unchecking the force mkv flag transcodes the file so the audio is present.

One other possibility, could we determine codec/container support on the client-side browser and send that down as a parameter to the stream request? That might side-step needing to set the flags at all.

bnkai · 2020-03-19T03:09:01Z

this needs to be added then

Audio codec
Is it worth the extra complexity / code to check for valid audio codec ? I didn't see any issues with files playing the video part and not the audio.

mkv supports a lot of audio codecs and ac3 is common in movies, browsers only support aac in mkv i think (not sure about opus or vorbis). If we need it then i have to extend the combo check to the audio codec also and transcode as needed.
I wouldn't want to add more to the client side. Using these flags is supposed to be a one time only depending on the browser one uses , a page refresh after the save is not that big of a deal for that imho.
Have you tried the problematic avi btw?
also https://github.com/henrypp/chromium/releases has windows builds of chromium that should support hevc

bnkai · 2020-03-20T18:31:01Z

Added checks for audio codec / player compatibility.
As a special case if MKV is forced as supported then when the media file is mkv and only its audio codec is not supported eg ac3 we copy the video stream as is, transcode the audio as opus and stream the media file as mkv(mime video/x-matroska) instead of webm. This results in less cpu usage and better video quality ( no video transcoding) for the specific case.

WithoutPants · 2020-03-20T21:46:22Z

A possible future iteration to investigate (not necessary for this PR): https://www.nomensa.com/blog/2011/detecting-browser-compatibility-html5-video-and-audio

And this stack overflow answer: https://stackoverflow.com/a/7451727/695786

Determine from the client-side if the video is directly streamable, if not then do request to stream.mp4?transcode=1

Forgot to mention what is actually working. This fixed streaming the files which were previously not working for me.

I'll do another review pass and test with the latest build as soon as I'm able.

bnkai · 2020-03-20T23:00:02Z

I think those solutions are too much trouble for little gain. They move the selection code from the server to the client. The extra code in the server side is used also to make "better" decisions for the transcode part ( copy or transcode a stream when needed instead of the whole video ) .
If and when needed we should probably checkout ways to invalidate the isStreamable query or make it non cachable.

…e/354

WithoutPants

Mostly style-based feedback now.

Seems to test well in my environment. Only wrinkle is that I don't think the MKV setting change impacts until I reloaded the page.

pkg/api/routes_scene.go

pkg/ffmpeg/ffprobe.go

pkg/ffmpeg/media_detection.go

pkg/manager/task_transcode.go

pkg/manager/utils.go

bnkai · 2020-04-09T09:09:28Z

The reload is only once though when you change the settings anyway. I didn't have to change / look back at the mkv settings since i started the PR for example.
I think all the "cache" issues we have in stash can be better addressed when apollo client gets to the next version https://blog.apollographql.com/previewing-the-apollo-client-3-cache-565fadd6a01e
A lot more fine grained control over cache.

WithoutPants

Looks and tests ok.

…live file streaming or transcoding (stashapp#384) * add forceMKV, forceHEVC config options * drop audio stream instead of trying to transcode for ffmpeg unsupported/unknown audio codecs

bnkai added the improvement Something needed tweaking. label Feb 29, 2020

WithoutPants requested changes Mar 3, 2020

View reviewed changes

pkg/api/routes_scene.go Outdated Show resolved Hide resolved

pkg/api/routes_scene.go Outdated Show resolved Hide resolved

bnkai changed the title ~~[WIP] Add detection of container/codec compatibility for live file streaming~~ Add detection of container/codec compatibility for live file streaming Mar 6, 2020

Add detection of container/codec compatibility for live file streaming

d469307

* take into account container in generate transcodes task * add container info to DB * increment appSchema to 5

bnkai force-pushed the issue/354 branch from d3dbfdd to d469307 Compare March 10, 2020 16:47

* fix seeking for some non streamable files

9631725

bnkai changed the title ~~Add detection of container/codec compatibility for live file streaming~~ [WIP] Add detection of container/codec compatibility for live file streaming Mar 11, 2020

WithoutPants added this to the Version 0.2.0 milestone Mar 13, 2020

bnkai added 3 commits March 13, 2020 18:14

* add forceMKV, forceHEVC config options to backend

c55f025

* add config forceMKV,forceHEVC to UI v2.0

06744ae

* add config forceMKV, forceHEVC to UI v2.5

a5a6126

* drop audio stream instead of trying to transcode for ffmpeg unsuppo…

5d28390

…rted/unknown audio codecs

bnkai changed the title ~~[WIP] Add detection of container/codec compatibility for live file streaming~~ Add detection of container/codec compatibility for live file streaming or transcoding Mar 13, 2020

* added temporary prints foe debugging

78975ea

bnkai changed the title ~~Add detection of container/codec compatibility for live file streaming or transcoding~~ [WIP]Add detection of container/codec compatibility for live file streaming or transcoding Mar 19, 2020

* add check for audio codec to determine if file is streamable

352cdf3

bnkai changed the title ~~[WIP]Add detection of container/codec compatibility for live file streaming or transcoding~~ Add detection of container/video_codec/audio_codec compatibility for live file streaming or transcoding Mar 20, 2020

bnkai added 6 commits March 30, 2020 11:34

* fix typo

bed2d0b

Merge remote-tracking branch 'upstream/develop' into issue/354

02ffe76

Merge remote-tracking branch 'upstream/develop' into issue/354

31fd9ee

Merge remote-tracking branch 'upstream/develop' into issue/354

9381de9

* increment schema to 6

3f2907a

Merge branch 'develop' of https://github.com/stashapp/stash into issu…

a2db83c

…e/354

WithoutPants requested changes Apr 8, 2020

View reviewed changes

bnkai added 2 commits April 9, 2020 11:39

cosmetic fixes

d982593

* more cosmetic

41f06da

* add comments

0a359c6

WithoutPants approved these changes Apr 9, 2020

View reviewed changes

WithoutPants merged commit d561730 into stashapp:develop Apr 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add detection of container/video_codec/audio_codec compatibility for live file streaming or transcoding #384

Add detection of container/video_codec/audio_codec compatibility for live file streaming or transcoding #384

bnkai commented Feb 25, 2020 •

edited by WithoutPants

Loading

bnkai commented Mar 2, 2020 •

edited

Loading

WithoutPants left a comment •

edited

Loading

bnkai commented Mar 6, 2020 •

edited

Loading

WithoutPants commented Mar 10, 2020

WithoutPants commented Mar 11, 2020

bnkai commented Mar 11, 2020

WithoutPants commented Mar 11, 2020

bnkai commented Mar 11, 2020 •

edited

Loading

bnkai commented Mar 13, 2020 •

edited

Loading

WithoutPants commented Mar 13, 2020

WithoutPants commented Mar 17, 2020

bnkai commented Mar 17, 2020

WithoutPants commented Mar 17, 2020

bnkai commented Mar 18, 2020 •

edited

Loading

WithoutPants commented Mar 19, 2020

bnkai commented Mar 19, 2020 •

edited

Loading

bnkai commented Mar 20, 2020

WithoutPants commented Mar 20, 2020 •

edited

Loading

bnkai commented Mar 20, 2020

WithoutPants left a comment

bnkai commented Apr 9, 2020

WithoutPants left a comment

Add detection of container/video_codec/audio_codec compatibility for live file streaming or transcoding #384

Add detection of container/video_codec/audio_codec compatibility for live file streaming or transcoding #384

Conversation

bnkai commented Feb 25, 2020 • edited by WithoutPants Loading

bnkai commented Mar 2, 2020 • edited Loading

WithoutPants left a comment • edited Loading

Choose a reason for hiding this comment

bnkai commented Mar 6, 2020 • edited Loading

WithoutPants commented Mar 10, 2020

WithoutPants commented Mar 11, 2020

bnkai commented Mar 11, 2020

WithoutPants commented Mar 11, 2020

bnkai commented Mar 11, 2020 • edited Loading

bnkai commented Mar 13, 2020 • edited Loading

WithoutPants commented Mar 13, 2020

WithoutPants commented Mar 17, 2020

bnkai commented Mar 17, 2020

WithoutPants commented Mar 17, 2020

bnkai commented Mar 18, 2020 • edited Loading

WithoutPants commented Mar 19, 2020

bnkai commented Mar 19, 2020 • edited Loading

bnkai commented Mar 20, 2020

WithoutPants commented Mar 20, 2020 • edited Loading

bnkai commented Mar 20, 2020

WithoutPants left a comment

Choose a reason for hiding this comment

bnkai commented Apr 9, 2020

WithoutPants left a comment

Choose a reason for hiding this comment

bnkai commented Feb 25, 2020 •

edited by WithoutPants

Loading

bnkai commented Mar 2, 2020 •

edited

Loading

WithoutPants left a comment •

edited

Loading

bnkai commented Mar 6, 2020 •

edited

Loading

bnkai commented Mar 11, 2020 •

edited

Loading

bnkai commented Mar 13, 2020 •

edited

Loading

bnkai commented Mar 18, 2020 •

edited

Loading

bnkai commented Mar 19, 2020 •

edited

Loading

WithoutPants commented Mar 20, 2020 •

edited

Loading