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

RESOLVED FIXED in Firefox 45

Status

()

Core
WebRTC: Audio/Video
P3
minor
Rank:
38
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: jesup, Assigned: jesup, NeedInfo)

Tracking

(Blocks: 1 bug)

Trunk
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

(Whiteboard: [getUserMedia])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
Blocks: 800579

Updated

5 years ago
Priority: -- → P3
Whiteboard: [WebRTC], [blocking-webrtc-]
(Assignee)

Comment 1

4 years ago
Created attachment 8365493 [details] [diff] [review]
Handle MJPEG decode errors (and log camera type acquired)
(Assignee)

Comment 2

4 years ago
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
Attachment #8365493 - Flags: review?(tterribe)
(Assignee)

Updated

4 years ago
Whiteboard: [WebRTC], [blocking-webrtc-] → [getUserMedia]
(Assignee)

Comment 3

4 years ago
Created attachment 8368175 [details] [diff] [review]
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)
(Assignee)

Updated

4 years ago
Attachment #8365493 - Attachment is obsolete: true
Attachment #8365493 - Flags: review?(tterribe)
(Assignee)

Comment 4

4 years ago
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).
Attachment #8368175 - Flags: review?(adam)

Comment 5

4 years ago
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.
Attachment #8368175 - Flags: review?(adam) → review+

Updated

3 years ago
backlog: --- → webRTC+
Rank: 38
If this hasn't landed or become moot, we should refresh and land it.
Assignee: nobody → rjesup
Flags: needinfo?(rjesup)
QA Contact: jsmith
We should land this if it's not moot

Comment 8

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

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a9f93ff23f70
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.