New issue
Advanced search Search tips

Issue 663503 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Use ffmpeg demuxer to detect HLS playback

Project Member Reported by tguilbert@chromium.org, Nov 8 2016

Issue description

WMPA verifies the presence of 'm3u8' in the media's URL in order to determine whether we are playing HLS media or not.

We need to delay the decision of when a MediaPlayerRenderer is created from before WMPI creation time, to 'after trying to open a context'. This will allow us to detect HLS more accurately than inspecting URLs for substrings. This also implies being able to change type of the RendererFactory used after WMPI's creation.

Using FfmpegGlue::OpenContext() to will also allow the proper container_names::MediaContainerName to reported to the Media.DetectedContainer UMA, addressing  crbug.com/650891  at the same time.

 
Status: Assigned (was: Untriaged)
This bug also covers the work needed to allow "hotswapping" renderers after WMPI creation (but it is unclear how hotswapping might work after pipeline has been started, if there is a need for a different DemuxerStreamProvider::Type (soon to be renamed to MediaResource::Type))
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70d2a00a72c50c344436f139b2a3f5c09ec337bf

commit 70d2a00a72c50c344436f139b2a3f5c09ec337bf
Author: tguilbert <tguilbert@chromium.org>
Date: Tue Apr 25 00:30:44 2017

Add RendererFactorySelector

There are 4 types of renderers that are used by WMPI directly, with a
potential fifth type in the works. Currently, a single RendererFactory
is chosen at WMPI creation time, and bound for the lifetime of WMPI.
However, there is a growing need to allow the smooth transition between
the use of different renderer types, after WMPI’s creation.

This CL introduces the RendererFactorySelector, which will allow us to
choose between different RendererFactory types at runtime. Its purpose
is to aggregate the signals and centralize the logic necessary to choose
which RendererFactory should be used when creating a new Renderer.

The interface is still extremelly simple, but it is expected to grow
as the number of concurrent RendererFactory types grows. In other words,
if there are 3+ renderer factories at play, some complexity is
inevitable, and the right place for that complexity to live is in the
RendererFactorySelector.

NOTE: The RendererFactorySelector uses a FactoryType enum as a key, in
order to avoid having to take dependencies on different (sometimes
platform specific) RendererFactories.

BUG= 663503 

Review-Url: https://codereview.chromium.org/2711153006
Cr-Commit-Position: refs/heads/master@{#466837}

[modify] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/base/BUILD.gn
[add] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/base/renderer_factory_selector.cc
[add] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/base/renderer_factory_selector.h
[add] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/base/renderer_factory_selector_unittest.cc
[modify] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/70d2a00a72c50c344436f139b2a3f5c09ec337bf/media/blink/webmediaplayer_impl_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75e2bf66ab653982229c116d96257daf71936b47

commit 75e2bf66ab653982229c116d96257daf71936b47
Author: tguilbert <tguilbert@chromium.org>
Date: Wed Apr 26 20:13:12 2017

Remove |use_fallback_path_| from WMPI

Currently, WMPI chooses whether to use a MediaResource::Type::URL based
off of the explicit flag |use_fallback_path_|. This flag is only set at
WMPI creation time, in RenderFrameImpl. If the final redirected URL is
an HLS playlist, we request that WMPI be reloaded.

This CL changes WMPI to chose the MediaResource::Type based off of
RendererFactory::GetRequiredMediaResourceType(). This allows us to get
rid of |use_fallback_path_| and to update RendererFactorySelector after
redirects, rather than reloading WMPI.

BUG= 663503 

Review-Url: https://codereview.chromium.org/2769153005
Cr-Commit-Position: refs/heads/master@{#467428}

[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/content/renderer/media/android/media_player_renderer_client_factory.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/content/renderer/media/android/media_player_renderer_client_factory.h
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/base/media_url_demuxer.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/base/renderer_factory.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/base/renderer_factory.h
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/base/renderer_factory_selector.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/base/renderer_factory_selector.h
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/base/renderer_factory_selector_unittest.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/75e2bf66ab653982229c116d96257daf71936b47/media/blink/webmediaplayer_impl.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9affbc15342e08bc0162756c9d1ec32fdf492fab

commit 9affbc15342e08bc0162756c9d1ec32fdf492fab
Author: tguilbert <tguilbert@chromium.org>
Date: Mon May 08 21:17:03 2017

Simplify RFI::CreateMediaPlayer

Since CourierRendererFactory (used to be called AdaptiveRendererFactory)
no longer takes a RendererFactory as a construction parameter, we can
clean up the logic in RFI::CreateMediaPlayer, so that is is simpler to
read.

BUG= 663503 

Review-Url: https://codereview.chromium.org/2822353003
Cr-Commit-Position: refs/heads/master@{#470129}

[modify] https://crrev.com/9affbc15342e08bc0162756c9d1ec32fdf492fab/content/renderer/render_frame_impl.cc

Cc: julien.isorce@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b6be3d570fd3f83666aa26b3067d0af51841aa2

commit 6b6be3d570fd3f83666aa26b3067d0af51841aa2
Author: Thomas Guilbert <tguilbert@chromium.org>
Date: Fri Aug 18 03:17:27 2017

Add demuxer based HLS detection

Currently, we detect HLS streams based off of the presence of the string
".m3u8" in the media's URL.

This CL removes URL based HLS detection, and replaces it with demuxer
based detection. This means that we inspect the first few bytes of the
file using container_names::DetermineContainer(). If we detect an HLS
manifest, we interrupt the flow of StartPipeline(), update the
RendererFactorySelector to use MediaPlayerRenderer and restart the
pipeline's initialization.

This change will also implicitly fix UMA metrics for HLS playback, since
we log the results of container_names::DetermineContainer().

Bug:  663503 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia01ae77c0c2aac8882e4884f1bc2675c45924290
Reviewed-on: https://chromium-review.googlesource.com/616205
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495448}
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/content/renderer/media/media_factory.cc
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/base/media_log.cc
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/base/pipeline_status.h
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/blink/webmediaplayer_util.cc
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/filters/ffmpeg_demuxer.cc
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/filters/ffmpeg_glue.cc
[modify] https://crrev.com/6b6be3d570fd3f83666aa26b3067d0af51841aa2/media/filters/ffmpeg_glue.h

Status: Verified (was: Assigned)

Sign in to add a comment