800564 - In libyuv, MJPGToI420() throws away the return code in many cases

Status: RESOLVED FIXED

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

Description

[Randell Jesup [:jesup] (needinfo me)]

int MJPGToI420() in libyuv:
media/webrtc/trunk/third_party/libyuv/source/convert.cc
Location:	line 1607

a bunch of cases in the code set 'ret' to the result of a conversion, but then it throws them away and returns 0 (success).

Upstream issue in libyuv/webrtc.org.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Handle MJPEG decode errors (and log camera type acquired)

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8365493 [details] [diff] [review]
Handle MJPEG decode errors (and log camera type acquired)

Very simple patch.  See bug 963907 for an example of why this is needed.  I've filed Issue 2847 at webrtc.org for this

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Handle MJPEG decode errors (and log camera type acquired)

refresh to apply on top of libyuv update.  Fix MJPEG decode to ARGB as well as I420 (to match upstream)

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8368175 [details] [diff] [review]
Handle MJPEG decode errors (and log camera type acquired)

The "ret ? 0 : 1;"'s are cribbed from upstream (what I suggested to them when I filed the bug).

Comment 5

[Adam Roach [:abr]]

Comment on attachment 8368175 [details] [diff] [review]
Handle MJPEG decode errors (and log camera type acquired)

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

Looks good, with very minor nits.

::: content/media/webrtc/MediaEngineWebRTCVideo.cpp
@@ +208,5 @@
>        }
>      }
>    }
> +  LOG(("chose cap type %d, %dx%d @%dfps",
> +       mCapability.rawType, mCapability.width, mCapability.height, mCapability.maxFPS));

Format strings should be %u rather than %d.

Since the underlying size of an enum is allowed to vary, please static_cast<unsigned int>(mCapability.rawType).

This line is 88 columns wide; consider wrapping to 80.

Comment 6

[Maire Reavy [:mreavy]]

If this hasn't landed or become moot, we should refresh and land it.

Comment 7

[Maire Reavy [:mreavy]]

We should land this if it's not moot

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a9f93ff23f70

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/a9f93ff23f70