GNOME Bugzilla – Bug 692461
codecparsers: vc1: fix bitplanes decoding (DIFF6 or NORM6)
Last modified: 2013-01-25 12:41:03 UTC
Created attachment 234318 [details] [review] codecparsers: vc1: fix bitplanes decoding (DIFF6 or NORM6) Here are two patches to fix decoding of DIFF6 or NORM6 bitplanes. Fix decoding of DIFF6 or NORM6 bitplanes with an odd number of lines (3x2 "horizontal" tiles). In this case, we have to skip the first line of macroblocks but <width> number of bytes was used to do so, instead of the actual <stride> size. Fix parsing of residual bytes. This is a two-step process. First, remaining colums of full vertical resolution (<height>) need to be processed. Next, remaining bytes in the first row can be processed, while taking into account the fact that we may have filled in the first columns already. So, this is not full horizontal resolution. I could squash them altogether, if needed. Though, conceptually, this is slightly different issues. In particular, this fixes the sample video from: https://bugzilla.gnome.org/show_bug.cgi?id=668565
Created attachment 234319 [details] [review] codecparsers: vc1: fix bitplanes decoding (DIFF6 or NORM6) [residual bytes]
SMPTE-421M Figure 89 (b) will help to correctly visualize how we need to handle residual bytes, as addressed in the second patch: (i) vertical leftovers first, and (ii) horizontal ones minus those that were parsed in (i).
I think you should feel to push further fixes to the VC-1 codec parser code without putting them in bugzilla first, as long as they don't change API :)
(In reply to comment #3) > I think you should feel to push further fixes to the VC-1 codec parser code > without putting them in bugzilla first, as long as they don't change API :) Yes, but I was wondering if squashing them was acceptable. They technically represent different issues but if I don't squash them into a single patch, git log --oneline will show two commits with the same subject. Just comsmetics, that doesn't disturb me too much otherwise. :)
If they are slightly different issues, leave them separate. Just do whichever you think is best.
commit 10639eb88972bf89656af694b8bf8c4b41255c69 Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com> Date: Thu Jan 24 18:14:28 2013 +0100 codecparsers: vc1: fix bitplanes decoding (DIFF6 or NORM6 residual bytes). Fix parsing of residual bytes. This is a two-step process. First, remaining colums of full vertical resolution (<height>) need to be processed. Next, remaining bytes in the first row can be processed, while taking into account the fact that we may have filled in the first columns already. So, this is not full horizontal resolution. The following figure helps in understanding the expected order of operations, for a 8x5 MBs bitplane. 5 5 6 6 6 6 6 6 5 5 1 1 1 2 2 2 5 5 1 1 1 2 2 2 5 5 3 3 3 4 4 4 5 5 3 3 3 4 4 4 So, after tiles 1 to 4 are decoded, vertical tile 5 needs to be processed (2x5 MBs) and then the horizontal tile 6 (6x1 MBs). https://bugzilla.gnome.org/show_bug.cgi?id=692461 Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com> commit fa2a526f04b1f51201fbb837fa430f93609938e8 Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com> Date: Thu Jan 24 17:28:22 2013 +0100 codecparsers: vc1: fix bitplanes decoding (DIFF6 or NORM6). Fix decoding of DIFF6 or NORM6 bitplanes with an odd number of lines (3x2 "horizontal" tiles). In this case, we have to skip the first line of macroblocks but <width> number of bytes was used to do so, instead of the actual <stride> size. This fixes decoding for the video sample attached to: https://bugzilla.gnome.org/show_bug.cgi?id=668565 https://bugzilla.gnome.org/show_bug.cgi?id=692461 Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>