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 726024 - Share more code between video decoder and encoder
Share more code between video decoder and encoder
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
unspecified
Other Linux
: Normal enhancement
: 1.2.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-10 09:55 UTC by deathsimple@vodafone.de
Modified: 2014-07-23 08:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (21.04 KB, patch)
2014-03-10 09:55 UTC, deathsimple@vodafone.de
committed Details | Review
Fix debug category initializatrion. (1.75 KB, patch)
2014-03-14 10:46 UTC, deathsimple@vodafone.de
committed Details | Review

Description deathsimple@vodafone.de 2014-03-10 09:55:42 UTC
Created attachment 271409 [details] [review]
Patch

Identical functionality spread of two different components.
We can't use a common base class because of different inheritance,
but let's try to share the code anyway.
Comment 1 Sebastian Dröge (slomo) 2014-03-12 07:52:41 UTC
Review of attachment 271409 [details] [review]:

Looks good, just some minor things

::: omx/gstomxvideo.c
@@ +1,3 @@
+/*
+ * Copyright 2014 Advanced Micro Devices, Inc.
+ *   Author: Christian König <christian.koenig@amd.com>

Copy over the copyright notices from the files from which you copied the code

::: omx/gstomxvideo.h
@@ +43,3 @@
+    GstVideoCodecState * state);
+
+GstCaps * gst_omx_video_get_caps_4_map(GList * map);

Please rename this to caps_for_map() :)
Comment 2 Sebastian Dröge (slomo) 2014-03-12 11:48:52 UTC
commit 6b28cf0378bc44c2f7a0db040af05409a05c3919
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Mar 12 12:48:12 2014 +0100

    omx: Copy old copyright notice into the new file

commit a04ef276e58523f0193c3d2a8cc75a2741324f6a
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Mar 12 12:47:34 2014 +0100

    omx: Rename function from _4_ to _for_ for clarity

commit 6bf4d9a498ca163a0c431e8a864658922a8bab1f
Author: Christian König <christian.koenig@amd.com>
Date:   Mon Mar 3 16:15:24 2014 +0100

    omxvideo: start sharing more code between video decoder and encoder
    
    Identical functionality spread of two different components.
    We can't use a common base class because of different inheritance,
    but let's try to share the code anyway.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726024
Comment 3 deathsimple@vodafone.de 2014-03-12 13:10:09 UTC
Actually just found a minor bug in the patch that isn't fixed yet. 

GST_DEBUG_CATEGORY_STATIC (gst_omx_video_debug_category);
#define GST_CAT_DEFAULT gst_omx_video_debug_category

Those lines cause errors/warnings when debugging is turned on because the debug category isn't initialized anywhere.

How should we fix that?
Comment 4 Sebastian Dröge (slomo) 2014-03-12 13:33:31 UTC
Oops. Initialise it in class_init :)
Comment 5 Sebastian Dröge (slomo) 2014-03-12 13:34:29 UTC
Erm, we're not talking about the bufferpool here... right. Initialise it in plugin_init() then and make it a extern/non-static debug category
Comment 6 deathsimple@vodafone.de 2014-03-14 10:46:07 UTC
Created attachment 271857 [details] [review]
Fix debug category initializatrion.

Ok, patch is attached. Please review and commit (or should I open up a new bug for this?).
Comment 7 Sebastian Dröge (slomo) 2014-03-15 11:42:34 UTC
commit 8a860bd02491237a6c636c79539786cf5110c6bd
Author: Christian König <christian.koenig@amd.com>
Date:   Thu Mar 13 14:26:58 2014 +0100

    omxvideo: fix debug category initialisation
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726024