1365227 - AVCC NAL converter should generate extradata.

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Jean-Yves Avenard [:jya]]

The AVCC converter converts the NAL from AnnexB to AVCC. However, it doesn't generate the related extradata.

Making further use of the converted data difficult.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1365227: P1. Create AVCC header for converted annexB.

Otherwise AnnexB::IsAVCC will return false on those converted samples.

Review commit: https://reviewboard.mozilla.org/r/139726/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139726/

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1365227: P2. Enforce that ExtractExtra data works on AVCC only.

Review commit: https://reviewboard.mozilla.org/r/139728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139728/

Comment 3

[James Cheng[:JamesCheng]]

Comment on attachment 8868117 [details]
Bug 1365227: P1. Create AVCC header for converted annexB.

https://reviewboard.mozilla.org/r/139726/#review143320

Awesome.

Comment 4

[James Cheng[:JamesCheng]]

Comment on attachment 8868118 [details]
Bug 1365227: P2. Enforce that ExtractExtra data works on AVCC only.

https://reviewboard.mozilla.org/r/139728/#review143322

lgtm.

Comment 5

[James Cheng[:JamesCheng]]

Thank you for solving this problem with such a simple logic.

I've also tested it with our HLS scenario.

It also works.

Thank you again for helping me.

Comment 6

[Jean-Yves Avenard [:jya]]

awesome.. glad that worked.

My apologies for giving you the wrong pointers earlier on...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02ce5318570c
P1. Create AVCC header for converted annexB. r=JamesCheng
https://hg.mozilla.org/integration/autoland/rev/26cf15cc5389
P2. Enforce that ExtractExtra data works on AVCC only. r=JamesCheng

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/02ce5318570c
https://hg.mozilla.org/mozilla-central/rev/26cf15cc5389

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8868118 [details]
Bug 1365227: P2. Enforce that ExtractExtra data works on AVCC only.

Only this patch should be uplifted 

Approval Request Comment
[Feature/Bug causing the regression]: unknown. Was discovered at random while working on something else. 
[User impact if declined]: some stream (likely BBC one) won't start. 
[Is this code covered by automated tests?]: in theory yes. But obviously we missed that bit. 
[Has the fix been verified in Nightly?]: NA
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: we incorrectly assumed that a given byte signature was indicating a type of content that can't happen yet. BBC had reported that some streams didn't start sometime this could be the reason. 
[String changes made/needed]: none

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8868118 [details]
Bug 1365227: P2. Enforce that ExtractExtra data works on AVCC only.

Fix an intermittent stream error for BBC which is important for users. Beta54+. Should be in 54 beta 10.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/93e66a0ce810

Comment 12

[Andrei Vaida [:avaida]]

(In reply to Jean-Yves Avenard [:jya] from comment #9) 
> [Is this code covered by automated tests?]: in theory yes. But obviously we
> missed that bit. 
> [Has the fix been verified in Nightly?]: NA
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on jya's assessment on manual testing needs.