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 690240 - encodebin: remove test of encoder name vs preset name
encodebin: remove test of encoder name vs preset name
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal critical
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-12-15 01:59 UTC by Alban Browaeys
Modified: 2012-12-18 16:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
encodebin: remove test of encoder name vs preset name (1.88 KB, patch)
2012-12-15 01:59 UTC, Alban Browaeys
none Details | Review
Patch to avoid changing the behaviour leting the user choose what preset element should be used (4.85 KB, patch)
2012-12-15 18:00 UTC, Thibault Saunier
committed Details | Review

Description Alban Browaeys 2012-12-15 01:59:41 UTC
Created attachment 231606 [details] [review]
encodebin: remove test of encoder name vs preset name

Commit 7358cba made use of the preset_name though it also added
a comparison between the preset name and the factory object name
(the encoder name). As those rarely matches the test fails for cheese
vp8enc factory name vs preset  "Profile Realtime".
Comment 1 Tim-Philipp Müller 2012-12-15 14:56:39 UTC
Just came accross this as well when trying the encodebin-using test case from bug #690132 . Not entirely sure the code makes sense in its present form. Also looks like it might break existing code. Raising priority for now.
Comment 3 Thibault Saunier 2012-12-15 16:59:25 UTC
Ok, so the idea is that the preset is the factory name of the GstElement that implement the GstPreset interface, and the preset_name is the actual preset name of the preset. I know it sensibly changes the behavior but it also lets real control to the user over what element should be used and what preset to apply on that element.

I can just revert that if the change in behavior is not acceptable.
Comment 4 Thibault Saunier 2012-12-15 17:01:44 UTC
Btw, I would just revert all my changes instead of applying the proposed patch as the patch makes my previous patches useless.
Comment 5 Thibault Saunier 2012-12-15 18:00:03 UTC
Created attachment 231631 [details] [review]
Patch to avoid changing the behaviour leting the user choose what preset element should be used
Comment 6 Alban Browaeys 2012-12-15 18:47:28 UTC
I think I got it right now . Thanks . 
Will send a patch to cheese with this change : "encoder name" as preset and "profile" as preset_name.
Sorry for the noise.
Comment 7 Thibault Saunier 2012-12-15 18:50:48 UTC
Alban I think I should rather apply my last patch as I should not have changed the behaviour. Tim what do you think?
Comment 8 Sebastian Dröge (slomo) 2012-12-17 13:04:52 UTC
Looks better
Comment 9 Thibault Saunier 2012-12-17 13:13:53 UTC
Ok, merged. Really sorry about that.

commit e79f0e801eccabe245b4ba956c036c4130ffc791
Author: Thibault Saunier <thibault.saunier@collabora.com>
Date:   Sat Dec 15 14:43:40 2012 -0300

    encodebing: Use the preset_name as the factory name and preset as the name of the preset
    
    The naming is not perfect, but at least we can keep the exact same behaviour as
    before.