New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758668 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Move MSE bitstream conversion to ChunkDemuxerStream (or to decoder wrappers?)

Project Member Reported by wolenetz@chromium.org, Aug 24 2017

Issue description

* Some codec types (h264, aac, etc) in some containers (eg, MP4) may need each "coded frame" to be converted (e.g. MP4 AVC h.264 -> Annex-B h.264) for use in decoders.
* But not all decoders require such conversion (eg, vtvda "unconverts" what mp4_stream_parser converts from avc h264 to annex-b h264)
* And actual choice of decoder may vary (fallbacks/etc)
* Decoders today signal the DemuxerStream w.r.t. conversion is needed via ::EnableBitstreamConverter()
* FFmpegDemuxerStream conditionally does the conversion based on that signal today.
* But ChunkDemuxerStream leaves it as a no-op because the MSE stream parsers *assume* conversion is always necessary.
* So we're doing extra work in some cases.
* Also, this "extra work" is fragile when the input coded frames don't strictly match expectations within the associated bitstream converter (See https://chromium-review.googlesource.com/c/chromium/src/+/630636, for example). It's unclear whether retaining strictness is required even when not performing conversion.

This bug tracks considering making the MSE version of a pipeline perform the conversion on-demand only when it's enabled on the demuxer stream.

Note, some of the coded frame's bitstream validation may still be desired at parse time to give advance notification of error or debug info during parsing, but that's not required.

Also note that EME-specific pieces like the decrypt_config updating of subsamples currently done in stream parsing, for example in mp4_stream_parser, will also need to be relocated to where the bitstream conversion is done.

(Thanks to sandersd@ for helping me frame this problem)
 
Cc: xhw...@chromium.org
cc+=xhwang@ since EME is involved (see OP).

Comment 2 by xhw...@chromium.org, Aug 28 2017

For "decrypt_config updating of subsamples currently done in stream parsing, ..., will also need to be relocated to where the bitstream conversion is done". Does this mean decrypting decoders should always return true in NeedsBitstreamConversion()? But that might not work in DecryptingDemuxerStream.
@#2, no, we just want to make MSE bitstream conversion conditional and on-demand, rather than always-on. To accomplish that, I'm thinking we need to move the conversion to ChunkDemuxerStream, and make ChunkDemuxerStream obey the presence (or lack of) a call to the ::EnableBitstreamConverter() to control whether or not the conversion happens (note, that interface seems a little weird to me: it only allows either default on disabled, or always-enabled once ::EnableBitstreamConverter() is called; but no reset once enabled?? -- what if a fallback decoder no longer needs conversion?)
Back to the EME case: current MSE "always convert" code relies on info in the decrypt_config to guide the conversion in the presence of encryption. The conversion also updates decrypt_config's subsamples because of ..um.. the conversion :)  So, when we make the conversion on-demand and done in chunkdemuxerstream, the decrypt_config will be needed there and could also be modified there due to the conversion.

Comment 4 by kqyang@chromium.org, Sep 26 2017

Cc: hmchen@chromium.org kqyang@chromium.org
I believe the current H264 decoder in CDM (CoreCodec) actually supports NAL unit stream. hmchen@, correct me if I am wrong. 

We may be able to save a "tiny" of CPU and power by not doing bitstream conversion. I don't expect a noticeable saving though on popular contents as the bitstream conversion is pretty efficient if NAL length size is the same as start code size, i.e. 4, while a NAL length size of 4 is usually used on most contents.

Summary: Move MSE bitstream conversion to ChunkDemuxerStream (or to decoder wrappers?) (was: Move MSE bitstream conversion to ChunkDemuxerStream)
Perhaps this bug isn't going far enough; the specific decoders know best if bitstream conversion is necessary, and maybe we should instead have them do such conversion. IIUC, sandersd@ has been considering this for a while, but I couldn't find the specific crbug for that (though I found this one).
Labels: -Pri-2 Pri-3

Sign in to add a comment