[FFmpeg-devel] [PATCH v3 1/3] avcodec/mjpegdec: fix non-subsampled RGB JPEGs

[Leo Izen]

On 4/19/23 14:58, Michael Niedermayer wrote:
> On Wed, Apr 19, 2023 at 02:11:24PM -0400, Leo Izen wrote:
>> The change introduced in b18a9c29713abc3a1b081de3f320ab53a47120c6
>> created a regression for non-subsampled progressive RGB jpegs. This
>> should fix that.
>> ---
>>   libavcodec/mjpegdec.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
>> index 01537d4774..1e3ddb72fb 100644
>> --- a/libavcodec/mjpegdec.c
>> +++ b/libavcodec/mjpegdec.c
>> @@ -1698,7 +1698,8 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask,
>>           s->h_scount[i]  = s->h_count[index];
>>           s->v_scount[i]  = s->v_count[index];
>>   
>> -        if(nb_components == 3 && s->nb_components == 3 && s->avctx->pix_fmt == AV_PIX_FMT_GBRP)
>> +        if((nb_components == 3 || nb_components == 1) && s->nb_components == 3
>> +                && s->avctx->pix_fmt == AV_PIX_FMT_GBRP && !s->progressive)
>>               index = (index+2)%3;
> 
> Why is progressive/!progressive special cased in all the new RGB code ?
> 

With progressive, I decode RGB in RGB-order, and then pivot it into 
GBR-order, whereas baseline is just decoded directly into GBR-order. If 
you decode progressive directly in GBR-order the buffers will be the 
wrong size and it will overrun the subsampled buffer when filling it 
with a non-subsampled one. See the allocation block on line 766 of 
mjpegdec.c. This depends on h_count and v_count, which cannot be changed 
or pivoted as if you do so, progressive JPEGs will fail to decode at all 
(invalid VLC entries, etc.)

Ideally, you'd just alloc them the right size, but s->component_index[i] 
won't refer to the right index for many progressive files, depending on 
whether the SOS marker has 1 or 3 components. If you have SOS markers 
with one component it will not properly pivot the colors.

Initially, I didn't have the checks and just always decoded in RGB order 
and then pivoted, but that broke some baseline files like the ones in 
Trac #4045. I used some casework so I could handle all files I tested 
with this.

If anyone has any suggestions on how to make the casework more elegant 
I'm all ears but this is the solution I found to work with every sample 
I tested.

- Leo Izen (Traneptora/thebombzen)