1304252 - Provide more detailed error for decoders part 2.

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Missed some decoders with bug 1303673..

complete the missing ones.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1304252: P1. Provide error details for ffmpeg decoder.

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

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1304252: P2. Provide further error details for android decoder.

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

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1304252: P3. Further error details for agnostic decoders.

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

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8793152 [details]
Bug 1304252: P1. Provide error details for ffmpeg decoder.

https://reviewboard.mozilla.org/r/79946/#review78678

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8793153 [details]
Bug 1304252: P2. Provide further error details for android decoder.

https://reviewboard.mozilla.org/r/79948/#review78682

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8793154 [details]
Bug 1304252: P3. Further error details for agnostic decoders.

https://reviewboard.mozilla.org/r/79950/#review78684

r+ with nit:

::: dom/media/platforms/agnostic/VorbisDecoder.cpp:180
(Diff revision 1)
>    }
>  
> -  if (vorbis_synthesis_blockin(&mVorbisDsp,
> -                               &mVorbisBlock) != 0) {
> -    return NS_ERROR_DOM_MEDIA_DECODE_ERR;
> +  err = vorbis_synthesis_blockin(&mVorbisDsp, &mVorbisBlock);
> +  if (err) {
> +    return MediaResult(NS_ERROR_DOM_MEDIA_DECODE_ERR,
> +                       RESULT_DETAIL("Vorbis synthesis blockin:%d", err));

"blockin" is not a real word.
How about "Vorbis synthesis block assembly error:%d"?

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8793154 [details]
Bug 1304252: P3. Further error details for agnostic decoders.

https://reviewboard.mozilla.org/r/79950/#review78684

> "blockin" is not a real word.
> How about "Vorbis synthesis block assembly error:%d"?

it's the name of the function causing the error. wouldn't this be preferred?

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8793154 [details]
Bug 1304252: P3. Further error details for agnostic decoders.

https://reviewboard.mozilla.org/r/79950/#review78684

> it's the name of the function causing the error. wouldn't this be preferred?

I was trying to be consistent with the previous error at line 174, where you seemed to transform the function name into a sentence related to the function meaning, and added "error".
In that case I think you should use the exact function name, and add "error", i.e.: "vorbis_synthesis_blockin error:%d".
Or something else as you see fit; I only object to "blockin" being used as an English noun when it is not. -- Well, apart from what the Urban Dictionary says, but I don't think it fits with the Vorbis function.

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8793154 [details]
Bug 1304252: P3. Further error details for agnostic decoders.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79950/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69738c146f7f
P1. Provide error details for ffmpeg decoder. r=gerald
https://hg.mozilla.org/integration/autoland/rev/4b9917e55cf3
P2. Provide further error details for android decoder. r=gerald
https://hg.mozilla.org/integration/autoland/rev/8a87c36b6596
P3. Further error details for agnostic decoders. r=gerald

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/69738c146f7f
https://hg.mozilla.org/mozilla-central/rev/4b9917e55cf3
https://hg.mozilla.org/mozilla-central/rev/8a87c36b6596

Comment 12

[Jean-Yves Avenard [:jya]]

Comment on attachment 8793152 [details]
Bug 1304252: P1. Provide error details for ffmpeg decoder.

Approval request is for all changes.

Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: Making harder to troubleshoot current decoding error seen with amazon and netflix
[Describe test coverage new/current, TreeHerder]: In central
[Risks and why]: None. We're just adding details to help identify where failures occur
[String/UUID change made/needed]: None

This is a followup on bug 1303673

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8793152 [details]
Bug 1304252: P1. Provide error details for ffmpeg decoder.

This patch provides more detailed error information for troubleshooting. Take it in 51 aurora.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/70ffb93aff31
https://hg.mozilla.org/releases/mozilla-aurora/rev/b21c8cde69ce
https://hg.mozilla.org/releases/mozilla-aurora/rev/0f63aa2bdcfb