GNOME Bugzilla – Bug 764370
codecparser: vp9: Wrong default values in non-intraonly/non-keyframe header fields
Last modified: 2016-04-15 11:45:49 UTC
The bit_depth, color_space, color_range, subsampling_x and subsampling_y fields in frame_headers will be present only when the frames are either keyframe or intra-only frame. The current code just assigning default values for these fields when the frame is nonkey/non-intraonly which is wrong. For eg: this will assign bitdepth=8 even fro 10-bit frames. Either we have to move these fields from FrameHdr structure to global GstVp9Parser structure. Which is no extra copy but with ABI change. OR We should keep the copies of all these fields in Private structure (GstVp9ParserPrivate), and copy them back to FrameHdr for each non-keyframe/non-intraonly frame. Not sure which one is preferred :)
Created attachment 325076 [details] [review] codecparser: vp9: move persistent fields from GstVp9FrameHdr to GstVp9Parser The subsampling_x, subsampling_y, bit_depth, color_space and color_range fileds are moved from GstVp9FrameHdr to the global GstVp9Parser structure. These fields are only present in keyframe or intra-only frame, no need to duplicate them for inter-frames. This is an ABI change. https://bugzilla.gnome.org/show_bug.cgi?id=764370
Created attachment 325081 [details] [review] codecparser: vp9: move persistent fields from GstVp9FrameHdr to GstVp9Parser There was a g_message() debug log in the previous patch ;), removed it
Had few regression testing, didn't find any issue. Also had few discussions about the desired approach in IRC. I am going to push this.
commit 88a3b4da3c297f9f59bc833f9fde19e6345cd435 Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com> Date: Fri Apr 1 14:12:08 2016 +0300 codecparser: vp9: move persistent fields from GstVp9FrameHdr to GstVp9Parser The subsampling_x, subsampling_y, bit_depth, color_space and color_range fileds are moved from GstVp9FrameHdr to the global GstVp9Parser structure. These fields are only present in keyframe or intra-only frame, no need to duplicate them for inter-frames. This is an ABI change. https://bugzilla.gnome.org/show_bug.cgi?id=764370
Review of attachment 325081 [details] [review]: pushed, closing.
Thanks. When closing bugs please also set the target_milestone field to the version that will include the fix, i.e. in this case 1.9.1 since it only landed in git master (if it was also cherry-picked into the 1.8 branch it would be 1.8.1 since that's the upcoming 1.8 release). That makes things easier for your release monke^Hmanagers.
(In reply to Tim-Philipp Müller from comment #6) > Thanks. When closing bugs please also set the target_milestone field to the > version that will include the fix, i.e. in this case 1.9.1 since it only > landed in git master (if it was also cherry-picked into the 1.8 branch it > would be 1.8.1 since that's the upcoming 1.8 release). That makes things > easier for your release monke^Hmanagers. Sure, will take care of it next time..thanks..