GNOME Bugzilla – Bug 792260
msdk: decode: Add mpeg2 decoder
Last modified: 2018-01-11 02:24:50 UTC
Enable mpeg2 decoder in MediaSDK plugin. Patch is pretty straightforward, just the matter of enabling.
Created attachment 366403 [details] [review] msdk: Add mpeg2 decdoer
Review of attachment 366403 [details] [review]: ::: sys/msdk/gstmsdkmpeg2dec.c @@ +48,3 @@ + "framerate = (fraction) [0/1, MAX], " + "width = (int) [ 1, MAX ], height = (int) [ 1, MAX ], " + "mpegversion = 2") This probably also handles mpegversion=1 or not? And you're missing systemstream=false here
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 366403 [details] [review] [review]: > > ::: sys/msdk/gstmsdkmpeg2dec.c > @@ +48,3 @@ > + "framerate = (fraction) [0/1, MAX], " > + "width = (int) [ 1, MAX ], height = (int) [ 1, MAX ], " > + "mpegversion = 2") > > This probably also handles mpegversion=1 or not? NO, Only version 2. > And you're missing systemstream=false here My fault! Thanks for the review
Created attachment 366510 [details] [review] msdk: Add mpeg2 decoder
(In reply to sreerenj from comment #3) > (In reply to Sebastian Dröge (slomo) from comment #2) > > Review of attachment 366403 [details] [review] [review] [review]: > > > > ::: sys/msdk/gstmsdkmpeg2dec.c > > @@ +48,3 @@ > > + "framerate = (fraction) [0/1, MAX], " > > + "width = (int) [ 1, MAX ], height = (int) [ 1, MAX ], " > > + "mpegversion = 2") > > > > This probably also handles mpegversion=1 or not? > > NO, Only version 2. That would be quite surprising. Did you try? MPEG1 is approximately a subset of MPEG2.
(In reply to Sebastian Dröge (slomo) from comment #5) > (In reply to sreerenj from comment #3) > > (In reply to Sebastian Dröge (slomo) from comment #2) > > > Review of attachment 366403 [details] [review] [review] [review] [review]: > > > > > > ::: sys/msdk/gstmsdkmpeg2dec.c > > > @@ +48,3 @@ > > > + "framerate = (fraction) [0/1, MAX], " > > > + "width = (int) [ 1, MAX ], height = (int) [ 1, MAX ], " > > > + "mpegversion = 2") > > > > > > This probably also handles mpegversion=1 or not? > > > > NO, Only version 2. > > That would be quite surprising. Did you try? MPEG1 is approximately a subset > of MPEG2. I haven't tried. But at least MSDK spec only advertising MFX_CODEC_MPEG2 Is it the exact subset? I don't think so, but may be I am mistaken. I don't remember exactly, but there were mpeg1 streams which weren't working correctly with gstreamer-vaapi.
(In reply to sreerenj from comment #6) > (In reply to Sebastian Dröge (slomo) from comment #5) > > (In reply to sreerenj from comment #3) > > > (In reply to Sebastian Dröge (slomo) from comment #2) > > > > Review of attachment 366403 [details] [review] [review] [review] [review] [review]: > > > > > > > > ::: sys/msdk/gstmsdkmpeg2dec.c > > > > @@ +48,3 @@ > > > > + "framerate = (fraction) [0/1, MAX], " > > > > + "width = (int) [ 1, MAX ], height = (int) [ 1, MAX ], " > > > > + "mpegversion = 2") > > > > > > > > This probably also handles mpegversion=1 or not? > > > > > > NO, Only version 2. > > > > That would be quite surprising. Did you try? MPEG1 is approximately a subset > > of MPEG2. > > I haven't tried. But at least MSDK spec only advertising MFX_CODEC_MPEG2 > Is it the exact subset? I don't think so, but may be I am mistaken. I don't > remember exactly, but there were mpeg1 streams which weren't working > correctly with gstreamer-vaapi. Maybe it was the parser issue (wrongly advertising mpeg version , or something like that). According to specification, mpeg2 decoder should be able to decode mpeg1 streams too. So shall we just enable it? :)
I did some testing, mpeg1 stream doesn't seem to be decodable with MediaSDK (at least with MediaServerStudioEssentials2017R3).
Comment on attachment 366510 [details] [review] msdk: Add mpeg2 decoder Ok, go for it then.
Review of attachment 366510 [details] [review]: Pushed: commit 7536d1209573c357cdf8b443aa8ad155cf74cf3c
Review of attachment 366510 [details] [review]: ::: sys/msdk/gstmsdkmpeg2dec.c @@ +40,3 @@ + +GST_DEBUG_CATEGORY_EXTERN (gst_msdkmpeg2dec_debug); +#define GST_CAT_DEFAULT gst_msdkmpeg2dec_debug I guess you forgot to add, for sake of completeness, the GST_DEBUG_CATEGORY_INIT of this category, though I'm not sure if it is used at all (as other debugging categories for decoders)
(In reply to Víctor Manuel Jáquez Leal from comment #11) > Review of attachment 366510 [details] [review] [review]: > > ::: sys/msdk/gstmsdkmpeg2dec.c > @@ +40,3 @@ > + > +GST_DEBUG_CATEGORY_EXTERN (gst_msdkmpeg2dec_debug); > +#define GST_CAT_DEFAULT gst_msdkmpeg2dec_debug > > I guess you forgot to add, for sake of completeness, the > GST_DEBUG_CATEGORY_INIT of this category, though I'm not sure if it is used > at all (as other debugging categories for decoders) Fixed, Thank you :)