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 784988 - openh264enc: allow to dynamically change bitrate
openh264enc: allow to dynamically change bitrate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.x
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-07-15 23:38 UTC by Nicola
Modified: 2017-07-17 23:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (5.65 KB, patch)
2017-07-15 23:38 UTC, Nicola
none Details | Review
simple application to test the patch (1009 bytes, text/x-csrc)
2017-07-15 23:39 UTC, Nicola
  Details
indentation fix (2.12 KB, patch)
2017-07-17 22:56 UTC, Nicola
committed Details | Review
allow to dynamically change bitrate/max bitrate (4.38 KB, patch)
2017-07-17 22:56 UTC, Nicola
committed Details | Review
add GST_PARAM_MUTABLE_PLAYING (1.58 KB, patch)
2017-07-17 22:58 UTC, Nicola
committed Details | Review

Description Nicola 2017-07-15 23:38:17 UTC
Created attachment 355694 [details] [review]
proposed patch

.
Comment 1 Nicola 2017-07-15 23:39:06 UTC
Created attachment 355695 [details]
simple application to test the patch
Comment 2 Sebastian Dröge (slomo) 2017-07-17 12:43:00 UTC
Comment on attachment 355694 [details] [review]
proposed patch

Looks good but can you do that in two patches, one only changing the indentation and the other doing the actual change?
Comment 3 Alessandro Decina 2017-07-17 13:00:13 UTC
Also GST_PARAM_MUTABLE_PLAYING is set but not checked anywhere (either check for it or remove it). And since you're at it it would be great if you did the same for max-bitrate so people don't trip when they realize one can be changed but not the other.
Comment 4 Olivier Crête 2017-07-17 14:15:59 UTC
(In reply to Alessandro Decina from comment #3)
> Also GST_PARAM_MUTABLE_PLAYING is set but not checked anywhere (either check
> for it or remove it).

I think GST_PARAM_MUTABLE_PLAYING is mostly for documentation, and this one means we can do it in any mode, so no need to check.

> And since you're at it it would be great if you did
> the same for max-bitrate so people don't trip when they realize one can be
> changed but not the other.

No flags means it can only be changed in NULL even though that's horribly documented.
Comment 5 Nicola 2017-07-17 22:56:15 UTC
Created attachment 355789 [details] [review]
indentation fix
Comment 6 Nicola 2017-07-17 22:56:53 UTC
Created attachment 355790 [details] [review]
allow to dynamically change bitrate/max bitrate
Comment 7 Nicola 2017-07-17 22:58:51 UTC
Created attachment 355791 [details] [review]
add GST_PARAM_MUTABLE_PLAYING

I think it is useful to see that the property can be changed in any state using gst-inspect, anyway I did a separate patch so you cannot push this one if you don't like it
Comment 8 Olivier Crête 2017-07-17 23:17:23 UTC
merged

commit e8f11615bd2ceede201398fd3b409dacadc4de5c (HEAD -> master, upstream/master)
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Tue Jul 18 00:52:03 2017 +0200

    openh264enc: set GST_PARAM_MUTABLE_PLAYING for bitrate/max-bitrate properties
    
    This way is documented that these properties can be changed in any state
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784988

commit 11e8cf92f27188cdda2c6690f7130106e90f21d8
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Tue Jul 18 00:49:12 2017 +0200

    openh264enc: allow to dynamically change bitrate
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784988

commit 20b5db0615caa3a21fd0bf0727b22d7246bb6699
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Tue Jul 18 00:36:27 2017 +0200

    openh264enc: fix indentation
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784988