Use FF_THREAD_SLICE threading mode in FFmpeg for MediaSource streams.

Use FF_THREAD_SLICE threading mode in FFmpeg for MediaSource streams.

FFmpeg may delay frames in FF_THREAD_FRAME mode, which affects
MediaSource streams. Use FF_THREAD_SLICE mode in that case. This may slow
down video decoding for streams that were encoded without slicing, as in
that case decoding will be happening on a single thread.

BUG=338529

[Sergey Ulanov, 2014-01-28 01:42:11]

I haven't added unittests yet. First I'd like to know what you think about this
approach.

[Ami GONE FROM CHROMIUM, 2014-01-28 07:17:09]

In principle I don't prefer _FRAME to _SLICE, and I suspect that if _SLICE is
legit to use for all MSE then it's legit to use for all <video>, period, and you
don't need the enum (just flip the default in ffmpegvideodecoder to be _SLICE).

Some questions that I think bear on the question of whether it's ok to
s/FRAME/SLICE/g:

- Do any/all of vimeo, youtube, hulu, netflix, dailymotion, amazon, play etc use
slice-based encoding?  I.e. what %age of <video> will effectively take a
utilization hit?  @sergeyu: are you able to figure this out?

- Do we stand by that "2 threads is better than 1" policy decision made oh so
long ago?  @scherkus: do you have enough perf test infra set up now to report
whether this would be a bad move or a good move?  (if it's known to not regress
metrics we care about, dropping a thread per <video> tag would be awesome!)

Anyway, I'm just a noise-maker; this is scherkus' call.

https://codereview.chromium.org/148423005/diff/1/content/renderer/media/webme...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/148423005/diff/1/content/renderer/media/webme...
content/renderer/media/webmediaplayer_impl.cc:1149: threading_mode));
git cl format

https://codereview.chromium.org/148423005/diff/1/media/filters/ffmpeg_video_d...
File media/filters/ffmpeg_video_decoder.cc (right):

https://codereview.chromium.org/148423005/diff/1/media/filters/ffmpeg_video_d...
media/filters/ffmpeg_video_decoder.cc:31: // performance benefits on older
machines such as P4s with hyperthreading.
Oh, wow, this is some vintage comment.
(since it was written, we've been bit by pages with hundreds of <video> tags
being surprised at thread-startup-failure and P4s aren't exactly top-of-mind
anymore)

https://codereview.chromium.org/148423005/diff/1/media/filters/ffmpeg_video_d...
File media/filters/ffmpeg_video_decoder.h (right):

https://codereview.chromium.org/148423005/diff/1/media/filters/ffmpeg_video_d...
media/filters/ffmpeg_video_decoder.h:35: // Each thread decode a single frame
and multiple frames may be decoded in
s/decode/decodes/

[scherkus (not reviewing), 2014-01-28 18:46:45]

On 2014/01/28 07:17:09, Ami Fischman wrote:
> In principle I don't prefer _FRAME to _SLICE, and I suspect that if _SLICE is
> legit to use for all MSE then it's legit to use for all <video>, period, and
you
> don't need the enum (just flip the default in ffmpegvideodecoder to be
_SLICE).
> 
> Some questions that I think bear on the question of whether it's ok to
> s/FRAME/SLICE/g:
> 
> - Do any/all of vimeo, youtube, hulu, netflix, dailymotion, amazon, play etc
use
> slice-based encoding?  I.e. what %age of <video> will effectively take a
> utilization hit?  @sergeyu: are you able to figure this out?

IIRC slide-based encodes aren't very prevalent. We could UMA it up to collect
some data. 
 
> - Do we stand by that "2 threads is better than 1" policy decision made oh so
> long ago?  @scherkus: do you have enough perf test infra set up now to report
> whether this would be a bad move or a good move?  (if it's known to not
regress
> metrics we care about, dropping a thread per <video> tag would be awesome!)

We do! Running locally:

./out/Release/media_perftests --gtest_filter=*MP4* --gtest_repeat=10 | awk
'/RESULT/ { t += $4; cnt += 1; } END { printf("Average over %d runs: %f\n", cnt,
t / cnt); }'

SLICE
Average over 10 runs: 3.313302

FRAME
Average over 10 runs: 3.943826

So we're getting ~19% bump. I believe the test file in question is *not*
slice-based.

[Sergey Ulanov, 2014-01-28 23:40:07]

On 2014/01/28 18:46:45, scherkus wrote:
> On 2014/01/28 07:17:09, Ami Fischman wrote:
> > In principle I don't prefer _FRAME to _SLICE, and I suspect that if _SLICE
is
> > legit to use for all MSE then it's legit to use for all <video>, period, and
> you
> > don't need the enum (just flip the default in ffmpegvideodecoder to be
> _SLICE).
> > 
> > Some questions that I think bear on the question of whether it's ok to
> > s/FRAME/SLICE/g:
> > 
> > - Do any/all of vimeo, youtube, hulu, netflix, dailymotion, amazon, play etc
> use
> > slice-based encoding?  I.e. what %age of <video> will effectively take a
> > utilization hit?  @sergeyu: are you able to figure this out?
> 
> IIRC slide-based encodes aren't very prevalent. We could UMA it up to collect
> some data. 

I've tried a some random videos on youtube and vimeo. None of them were sliced.
Also VP9 doesn't support slicing at all. I.e. in most cases ffmpeg ends up using
only one thread with FF_THRAD_SLICE. Beside that YouTube uses MediaSource (i.e.
handling MediaSource streams differently may not be the right thing to do).


>  
> > - Do we stand by that "2 threads is better than 1" policy decision made oh
so
> > long ago?  @scherkus: do you have enough perf test infra set up now to
report
> > whether this would be a bad move or a good move?  (if it's known to not
> regress
> > metrics we care about, dropping a thread per <video> tag would be awesome!)
> 
> We do! Running locally:
> 
> ./out/Release/media_perftests --gtest_filter=*MP4* --gtest_repeat=10 | awk
> '/RESULT/ { t += $4; cnt += 1; } END { printf("Average over %d runs: %f\n",
cnt,
> t / cnt); }'
> 
> SLICE
> Average over 10 runs: 3.313302
> 
> FRAME
> Average over 10 runs: 3.943826
> 
> So we're getting ~19% bump. I believe the test file in question is *not*
> slice-based.

Right, that video is not sliced. On my machine I see even bigger difference 2.1
vs 3.3 runs/sec.
But I think that doesn't prove that threading decoding is actually beneficial.
One of the goals should be to minimize CPU consumption, and threading will
increase total CPU consumption. I think the only case when threading may be
useful is on CPUs where a single core is not powerful enough to decode every
frame on time and so the load needs to be spread between multiple cores. I don't
have a feel of how often that's the case. Also normally chrome needs to spend
time on YUV conversion and scaling, which would normally happen on a different
thread, so we are already using more than one core. I.e. threaded decoding may
not actually change anything. I'll try running it on a low-end machine and see
if it makes any difference.

[Ami GONE FROM CHROMIUM, 2014-01-29 00:43:12]

On Tue, Jan 28, 2014 at 3:40 PM, <sergeyu@chromium.org> wrote:

> [...] I'll try running it on a low-end machine and see if it makes any
> difference.
>

Cool, I think we're all on the same page and more data will inform a better
decision.

I think the ultimate trump card in this conversation is power-consumption.
 If you could argue that removing multithreaded decode used less power
while still allowing decode to keep up, that might be the most compelling
argument of all (vs. e.g. cpu utilization, etc).


https://codereview.chromium.org/148423005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[scherkus (not reviewing), 2014-01-29 01:12:59]

On 2014/01/29 00:43:12, Ami Fischman wrote:
> On Tue, Jan 28, 2014 at 3:40 PM, <mailto:sergeyu@chromium.org> wrote:
> 
> > [...] I'll try running it on a low-end machine and see if it makes any
> > difference.
> >
> 
> Cool, I think we're all on the same page and more data will inform a better
> decision.
> 
> I think the ultimate trump card in this conversation is power-consumption.
>  If you could argue that removing multithreaded decode used less power
> while still allowing decode to keep up, that might be the most compelling
> argument of all (vs. e.g. cpu utilization, etc).

Yeah! To be clear I'm up for re-assessing the decision. I agree that other
factors such as minimizing power consumption, my ever-favourite reducing # of
threads (see https://crbug.com/239925), not to mention eliminating decoder delay
may be more relevant than the initial "faster decode fps = better" goal we set
back then.

Heck, I still have my trusty old dual-core winxp laptop that I used for the
original benchmarking :)

[Sergey Ulanov, 2014-04-04 19:03:25]

I've experimented it bit more - it looks like this change does affect negatively
performance on some videos, particularly when one core can't keep up decoding.
I'm closing this CL for now.

[scherkus (not reviewing), 2014-04-04 20:14:40]

On 2014/04/04 19:03:25, Sergey Ulanov wrote:
> I've experimented it bit more - it looks like this change does affect
negatively
> performance on some videos, particularly when one core can't keep up decoding.
> I'm closing this CL for now.

Thanks for investigating! Looking forward to chatting this afternoon to explore
some more options in detail.