GNOME Bugzilla – Bug 582569
GstBaseDecode class
Last modified: 2011-05-16 19:42:52 UTC
I've been working on a GstBaseDecode class which purpose would be to refactor common code for decoder plugins. It is not specialized for video or audio, so maybe GstBaseAudioDecode and GstBaseVideoDecode could also be created inheriting from this. I do not have any experience writing decoder plugins (and very little experience writing gstreamer plugins in general), so this was more like an experiment to get into the details of how decoder plugins work. I analized the decoder plugins for ac3, mpeg2 and mad from -ugly, identified common bits and tried to put them together in this class. Probably there are tons of other things that an experienced decoder developer could add on top of what I did. Also, there might be some design issues with my proposal. Of course, this is very preliminary code, it needs polishing here and there, etc, but I thought it might be a good starting place to get feedback or to let some other more expererinced developer take it from here. I'll attach the implementation soon.
Created attachment 134625 [details] gstbasedecode.c The GstBaseDecode class
Created attachment 134626 [details] gstbasedecode.h GstBaseDecode header file
Just some random thought, I didn't look closer yet... but why is there a start/stop vfunc and init/reset? All other base classes only have start/stop so what would be the use case of init/reset that can't be done with start/stop? :) Other than that, great work :) I hope I can take a closer look at this in the next days... also you should probably take a look at the video decoder/encoder base classes that were added some days ago to GIT, they should probably be merged with your basedecode.
About the start/stop - init/reset stuff, looking at mpeg2dec for example, I see they do different stuff in NULL_TO_READY and READY_TO_PAUSED. Same for PAUSED_TO_READY and READY_TO_NULL, so that's why I thought 4 functions were needed. For other's, like mad, this is not the case and only two of them would be needed.
(In reply to comment #3) > also you should probably take a look at the video decoder/encoder > base classes that were added some days ago to GIT, they should probably be > merged with your basedecode. I guess you mean these: http://diracvideo.org/git?p=schroedinger.git;a=tree;f=gst-libs/gst/video;h=e62b635e73485658a817d5b88ddd2cbd245a7e7d;hb=HEAD but can't find them in gstreamer git though :-? I'll have a go at this and see what I can come up with.
I'm talking about bug #583045 The base classes are in gst-plugins-base/gst-libs/gst/video
And the base classes have now been moved to -bad :)
I was devoting some time to the OpenCore based AMR plugins, but I did not forget about this. I created this project in gitorious: http://gitorious.org/gst-plugins-bad-audio-codec-base-classes My idea is to do for audio something similar to what has been done for video and then see how much code can actually be shared between both. I will also see how my previous attempt of a basedecoder class fits in there. I'll be on vacations the next two weeks and I won't have Internet access, but I'll work on this offline. Hopefully I will have something by the 1st of Sept.
Ok, so I created this: http://gitorious.org/gst-plugins-bad-audio-codec-base-classes/gst-plugins-bad-audio-codec-base-classes/trees/master/gst-libs/gst/audio which contains two classes: GstBaseAudioCodec and GstBaseAudioDecoder, being the latter a subclass of the former. The original idea was to mimic what we have for video, but now I am not sure if this makes sense, as I feel like GstBaseAudioCodec could be enough. Right now, all the important behavior is in GstBaseAudioCodec. Probably it is just my lack of experience, I guess more experienced codec developers can point out differences in the behavior of encoders and decoders that justify having such subclasses of GstBaseAudioCodec, just tell me about them and I'll implement it. I added gtk-doc documentation so reviewers should have an easier time understanding the decisions I made when implementing this. I tested this implementation with the AMR-NB decoder and it worked well for me. I guess I should give it a try with more complex decoders. Also, I should probably try an encoder and see if GstBaseAudioCodec alone works well for that case too. As a side note, if you look at GstBaseAudioDecoder you'll see that it does not implement any decoder specific stuff (this is handled all at GstBaseAudioCodec, and hence the reason I doubt that subclasses of this are needed). Currently GstBaseAudioDecoder just implements a few source events and query functions, but I am not even sure if that's actually necessary/useful. Actually, I think most of these are handled by other elements in the pipeline (demuxers I guess), so maybe it is not worth the effort. So now I would like to have someone reviewing this and giving some feedback so I can add/fix whatever needs to be added or fixed :)
I also reworked the mad decoder plugin from -ugly to extend from my base class and it seems to work fine, so I guess that at least from a functional point of view, this base classes are good _enough_ to write decoders, even if they can be improved. I'll try to see how they work for encoders soon.
Moving to gst-plugins-bad component.
So, as mentioned in bug #642690, I'll be going forward with (moderately heavily) tweaking these classes to go alongside base audio encoder in aforementioned bug. Outcome will probably consist of 2 separate classes (audio encoder and audio decoder) along with some common helper structure/routines.
See #bug 642690 for result so far, and subsequent progress.
Mark, have you merged Iago's code with yours ? Just wonderin whether this bug should remain open or marked as a duplicate of #bug 642690
It's incorporated in the base audio stuff in bug #642690.
Ok, let's merge those two then (even if this one was first).. *** This bug has been marked as a duplicate of bug 642690 ***