Closed
Bug 853306
Opened 8 years ago
Closed 8 years ago
Make the gstreamer media backend play whitelisted codecs only
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: alessandro.d, Assigned: alessandro.d)
References
Details
Attachments
(1 file, 3 obsolete files)
8.98 KB,
patch
|
Details | Diff | Splinter Review |
As discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=794282, the gstreamer backend needs to support whitelisted codecs only. Patch already feedback+'d in https://bugzilla.mozilla.org/show_bug.cgi?id=794282#c60
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #727519 -
Flags: review?(cpearce)
Assignee | ||
Comment 2•8 years ago
|
||
Same patch with... + if (strstr (klass, "Demuxer") && !strstr(klass, "Metadata")) ...a check added to skip metadata parser elements while looking for container elements in the pipeline. Fixes regression with mp3s with id3 tags.
Attachment #727519 -
Attachment is obsolete: true
Attachment #727519 -
Flags: review?(cpearce)
Assignee | ||
Updated•8 years ago
|
Attachment #727525 -
Flags: review?(cpearce)
Assignee | ||
Comment 3•8 years ago
|
||
Hrm, sorry for the noise with the previous patches. This is the final patch, promised. Fixes m4a tests with: - {"audio/x-m4a", "audio/mpeg, mpegversion=(int)4"}, + {"audio/x-m4a", "audio/x-m4a"},
Attachment #727525 -
Attachment is obsolete: true
Attachment #727525 -
Flags: review?(cpearce)
Attachment #727564 -
Flags: review?(chris.double)
Comment 4•8 years ago
|
||
Comment on attachment 727564 [details] [diff] [review] fix mp4 caps Review of attachment 727564 [details] [diff] [review]: ----------------------------------------------------------------- Confirming that tests pass for me on Linux with this patch and the one from bug 853306 applied. Thanks Alessandro!
Attachment #727564 -
Flags: feedback+
Comment 5•8 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #4) > Confirming that tests pass for me on Linux with this patch and the one from > bug 853306 applied. I mean bug 853325... ;)
Comment 6•8 years ago
|
||
Comment on attachment 727564 [details] [diff] [review] fix mp4 caps Review of attachment 727564 [details] [diff] [review]: ----------------------------------------------------------------- Just minor style changes. r+ with those. ::: content/media/gstreamer/GStreamerFormatHelper.cpp @@ +53,5 @@ > GStreamerFormatHelper::GStreamerFormatHelper() > : mFactories(nullptr), > mCookie(static_cast<uint32_t>(-1)) > { > + unsigned int i; Remove this and declare in each for statement. @@ +56,5 @@ > { > + unsigned int i; > + > + mSupportedContainerCaps = gst_caps_new_empty(); > + for (i = 0; i < G_N_ELEMENTS(mContainers); i++) { for(unsigned int i = 0; ... @@ +63,5 @@ > + gst_caps_append(mSupportedContainerCaps, caps); > + } > + > + mSupportedCodecCaps = gst_caps_new_empty(); > + for (i = 0; i < G_N_ELEMENTS(mCodecs); i++) { for(unsigned int i = 0; ... ::: content/media/gstreamer/GStreamerReader.cpp @@ +296,5 @@ > break; > } > } > > + if (!NS_FAILED(ret)) Use NS_SUCCEEDED @@ +353,5 @@ > + GstElement* element; > + GstElementFactory* factory; > + const char* klass; > + GstCaps* caps; > + GstPad* pad; Declare these on first use/initialization where possible. @@ +356,5 @@ > + GstCaps* caps; > + GstPad* pad; > + > + GstIterator *it = gst_bin_iterate_recurse(GST_BIN(mPlayBin)); > + GstIteratorResult res; Declare this inside the loop at first use. @@ +357,5 @@ > + GstPad* pad; > + > + GstIterator *it = gst_bin_iterate_recurse(GST_BIN(mPlayBin)); > + GstIteratorResult res; > + while(!done) { Space needed between 'while' keyword and '('. @@ +358,5 @@ > + > + GstIterator *it = gst_bin_iterate_recurse(GST_BIN(mPlayBin)); > + GstIteratorResult res; > + while(!done) { > + res = gst_iterator_next(it, (void **)&element); so this becomes: while (!done) { GstIteratorResult res = gst_iterator_next... @@ +399,5 @@ > + } > + > + if (unsupported) > + return NS_ERROR_FAILURE; > + return NS_OK; Replace the conditional/return with: return unsupported ? NS_ERROR_FAILURE : NS_OK;
Attachment #727564 -
Flags: review?(chris.double) → review+
Assignee | ||
Comment 7•8 years ago
|
||
Attachment #727564 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 9•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2209aa5267f5
Assignee: nobody → alessandro.d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment 10•8 years ago
|
||
I don't suppose there is a pref here to enable playback of non-whitelisted codecs? :(
Comment 11•8 years ago
|
||
There is not. That's sort of the point. :/
You need to log in
before you can comment on or make changes to this bug.
Description
•