979812 - [MediaEncoder] VP8TrackEncoder::PrepareRawFrame should validate input image

Status: RESOLVED FIXED

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

Description

[u459114]

Attachment: TestVideoTrackEncoder.cpp

Attach video track test case draft.

Test case crash at #61
91   EXPECT_TRUE(NS_FAILED(encoder.GetEncodedTrack(container)));

http://dxr.mozilla.org/mozilla-central/source/content/media/encoder/VP8TrackEncoder.cpp#265
We should call data->IsValid() to confirm this image data is ok for encoding.

Comment 1

[Benjamin Chen [:bechen]]

If the data is invalid, should we treat it as a muted frame?

Comment 2

[u459114]

(In reply to Benjamin Chen [:bechen] from comment #1)
> If the data is invalid, should we treat it as a muted frame?
You can recover this error by turning that frame into muted(black frame) or just return error to the client side.
Unless we do encounter this problem in real use cases, I would suggest return error directly.

Comment 3

[Benjamin Chen [:bechen]]

Attachment: bug-979812.patch

Comment 4

[Ralph Giles (:rillian)]

Comment on attachment 8387401 [details] [diff] [review]
bug-979812.patch

Review of attachment 8387401 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/encoder/VP8TrackEncoder.cpp
@@ +290,5 @@
>      // Big-time assumption here that this is all contiguous data coming
>      // from getUserMedia or other sources.
>      MOZ_ASSERT(yuv);
> +    if (!yuv->IsValid()) {
> +      VP8LOG("PlanarYCbCrImage is not valid\n");

If this is something which shouldn't happen outside the unit test, NS_WARNING would be better here so it's more obvious when it goes wrong.

If sources can produce this in normal conditions, VP8LOG is better to avoid console spew.

Comment 5

[Benjamin Chen [:bechen]]

Attachment: bug-979812.patch

r=rillian.
Change VP8LOG to NS_WARNING.
try server: https://tbpl.mozilla.org/?tree=Try&rev=c078c9c4fb19

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/759f74aa389d

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/759f74aa389d