GNOME Bugzilla – Bug 698727
mulawenc: send maximum bitrate tag downstream
Last modified: 2013-04-25 11:23:33 UTC
Created attachment 242315 [details] [review] patch the maximum bitrate tag is needed downstream for creating a correct sdp. this patch introduces this functionality.
this is fixed bitrate, might make sense to use GST_TAG_BITRATE instead
I will have to rework the patch anyway because it should really be a gstaudioencoder. The reason is that we might have tags coming in on the sink and doing like in the patch would simply overwrite those. The audioencoder base element handles this with some smarts and redoing that work in mulawenc doesn't make sense. However, you are probably right that we should set GST_TAG_BITRATE as well, but we will need the MAX, so perhaps set all three of them (MIN, MAX and GST_TAG_BITRATE)?
Created attachment 242388 [details] [review] Patch # 2 Change mulawenc to gstaudioencoder and add tag support (setting min, max and normal bitrate)
I think there are some missing unrefs in this version. I'll fix it (In reply to comment #3) > Created an attachment (id=242388) [details] [review] > Patch # 2 [details] > > Change mulawenc to gstaudioencoder and add tag support (setting min, max and > normal bitrate)
I think the only unref missing here is that of the taglist of gst_audio_encoder_merge_tags(). Looks good otherwise. Do you want to do the same for the alaw encoder too and maybe the decoders?
Created attachment 242394 [details] [review] #3 same as #2 but with unref of taglist
(In reply to comment #5) > I think the only unref missing here is that of the taglist of > gst_audio_encoder_merge_tags(). Looks good otherwise. > > Do you want to do the same for the alaw encoder too and maybe the decoders? I can fix the others too... But I'll open a separate ticket for that if that's fine? This element I need for work, the others I will have to fix with lower prio. But I agree that it's for the best to keep them similar.
Thanks, pushed that :) Yes, please open new bugs for the others if you intend to work on that. commit fb0384fa0d8196bf30151da58bd033fa55c04dd4 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Thu Apr 25 12:44:15 2013 +0200 mulaw: Some minor memleak fixes and cleanup commit f0edb5fb7063e41d3d0c51bb8ec4b514da2e3c1e Author: Alexander Schrab <alexas@axis.com> Date: Wed Apr 24 13:56:56 2013 +0200 mulawenc: change to gstaudioencoder base, added bitrate tags
(In reply to comment #8) > Thanks, pushed that :) Yes, please open new bugs for the others if you intend > to work on that. > > commit fb0384fa0d8196bf30151da58bd033fa55c04dd4 > Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> > Date: Thu Apr 25 12:44:15 2013 +0200 > > mulaw: Some minor memleak fixes and cleanup > > commit f0edb5fb7063e41d3d0c51bb8ec4b514da2e3c1e > Author: Alexander Schrab <alexas@axis.com> > Date: Wed Apr 24 13:56:56 2013 +0200 > > mulawenc: change to gstaudioencoder base, added bitrate tags Great thanks. I'll get back to you about the other elements