Closed
Bug 834323
Opened 11 years ago
Closed 11 years ago
Consider to remove hardcoded samplerate from gstreamer pipeline.
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: romaxa, Assigned: romaxa)
Details
Attachments
(1 file)
1.75 KB,
patch
|
alessandro.d
:
review+
rillian
:
feedback+
|
Details | Diff | Splinter Review |
I found one issue with gstreamer backend, that we have hardcoded rate value... in that case if content does not match to that value, then gstreamer perform re-sampling which is kindof expensive... I would suggest to remove that rate value from gst description.
Assignee | ||
Comment 1•11 years ago
|
||
Comment 2•11 years ago
|
||
Comment on attachment 705940 [details] [diff] [review] Remove hardcoded rate from gstreamer pipeline Patch does what you said it does, but I have no idea how safe this is. Passing review to Alessandro, the original author. How many input sample rates have you tried? Does it work with 96 kHz? 8 kHz? 22.050?
Attachment #705940 -
Flags: review?(giles)
Attachment #705940 -
Flags: review?(alessandro.d)
Attachment #705940 -
Flags: feedback+
Assignee | ||
Comment 3•11 years ago
|
||
I tried only 44100, and 48000
Comment 4•11 years ago
|
||
Looks good: r+ (i don't have bugzilla powers to change the flag). IIRC it was hardcoded to 44100 because that's what the downstream code was expecting at the time I wrote the original patch.
Comment 5•11 years ago
|
||
(In reply to Alessandro Decina from comment #4) > Looks good: r+ (i don't have bugzilla powers to change the flag). Do you get an error when you try? I checked that you have the correct permissions and you seem to.
Comment 6•11 years ago
|
||
(In reply to Chris Double (:doublec) from comment #5) > Do you get an error when you try? I checked that you have the correct > permissions and you seem to. He should now. Gavin fixed the perms earlier today.
Updated•11 years ago
|
Attachment #705940 -
Flags: review?(alessandro.d) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e4f468b4906
Keywords: checkin-needed
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6e4f468b4906
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•