After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 698727 - mulawenc: send maximum bitrate tag downstream
mulawenc: send maximum bitrate tag downstream
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.x
Other Linux
: Normal enhancement
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-04-24 12:04 UTC by Alexander Schrab
Modified: 2013-04-25 11:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (7.48 KB, patch)
2013-04-24 12:04 UTC, Alexander Schrab
none Details | Review
Patch # 2 (17.34 KB, patch)
2013-04-25 05:50 UTC, Alexander Schrab
none Details | Review
#3 (17.38 KB, patch)
2013-04-25 08:42 UTC, Alexander Schrab
committed Details | Review

Description Alexander Schrab 2013-04-24 12:04:57 UTC
Created attachment 242315 [details] [review]
patch

the maximum bitrate tag is needed downstream for creating a correct sdp. this patch introduces this functionality.
Comment 1 Olivier Crête 2013-04-24 15:58:10 UTC
this is fixed bitrate, might make sense to use GST_TAG_BITRATE instead
Comment 2 Alexander Schrab 2013-04-24 17:07:27 UTC
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)?
Comment 3 Alexander Schrab 2013-04-25 05:50:49 UTC
Created attachment 242388 [details] [review]
Patch # 2

Change mulawenc to gstaudioencoder and add tag support (setting min, max and normal bitrate)
Comment 4 Alexander Schrab 2013-04-25 06:49:38 UTC
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)
Comment 5 Sebastian Dröge (slomo) 2013-04-25 07:49:33 UTC
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?
Comment 6 Alexander Schrab 2013-04-25 08:42:56 UTC
Created attachment 242394 [details] [review]
#3

same as #2 but with unref of taglist
Comment 7 Alexander Schrab 2013-04-25 08:45:35 UTC
(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.
Comment 8 Sebastian Dröge (slomo) 2013-04-25 10:45:35 UTC
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
Comment 9 Alexander Schrab 2013-04-25 11:23:33 UTC
(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