[Ffmpeg-devel] Last jpeg patch to support multiple-scan JPEGs

[Cyril Russo]

Kostya a ?crit :
> On Mon, Nov 20, 2006 at 06:56:08PM +0100, Cyril Russo wrote:
>   
>> Michael Niedermayer a ?crit :
>>     
>>> Hi
>>>
>>> On Mon, Nov 20, 2006 at 02:26:34PM +0100, Cyril Russo wrote:
>>>  
>>>       
>>>> Hi,
>>>>
>>>>
>>>>    
>>>>         
>>>>>> Worse, the patch from Kostya doesn't solve the multiple scan JPEG   
>>>>>> decoding errors as (s)he pretends. The problem is fully unrelated.
>>>>>> Interleaved JPEG != Progressive JPEG != Multiple scan JPEG
>>>>>>        
>>>>>>             
>>>>>  kostya posted a second patch which adds Multiple scan JPEG sipport
>>>>>  by removing 6 lines and chagning 2, while yours duplicates a whole 
>>>>>      
>>>>>           
>>>>  function (which btw ive complained about in my review and you ignored
>>>>    
>>>>         
>>>>>  it)
>>>>>      
>>>>>           
>>>>   
>>>> No, it's wrong. Kostya's patch doesn't add multiple scan JPEG support. 
>>>> He (I make the assumption he is a male)
>>>> added progressive support. There is a big difference with multiple scan 
>>>> support.
>>>>         
>
> Actually, progressive JPEG uses multiple scans too so after integrating it
> was easy to add proper support for multiple scans.
>
>   
>>>>    
>>>>         
>>> kostya did send 2 patches one for progressive and one for multiple scan
>>> jpeg
>>>
>>>
>>>  
>>>       
>>>> Again, try to decode the JPEG I've posted before with kostya version, it 
>>>> will fail.
>>>>    
>>>>         
>>> kostya said it works so ill trust him considering that you seem to be
>>> a little confused about which patch i speak off
>>>
>>> [...]
>>>  
>>>       
>> Ok, let's make things clear. I, and I suppose, all of us too, want to be 
>> able to decode all standard JPEGs.
>> I've found both patch from kostya, and only the last one got applied.
>> Anyway, the previous patch from /Mon Nov 20 06:51:15 CET 2006 /doesn't 
>> work either (the index in the scan is not used for decoding the scan 
>> while it should)/
>>     
>
> No, you have missed it (it was posted in your thread 'Incorrect JPEG decoding
> with patch').
>
>   
>> /Of course you can trust anyone older than me, I'm new here, but I'm not 
>> new in image processing.
>> Again, if you don't want to believe me, believe yourself by compiling 
>> lastest kostya version, and launch :
>> wget http://xryl669ny.free.fr/MyFile000.jpg && mv MyFile000.jpg 
>> test.mjpeg && ffmpeg -i test.mjpeg -vframes 1 -f mjpeg output.jpg
>>
>> You'll get a very beautiful (and uniform) grey picture in output.
>>     
>
> Try SVN r7143, it is already there.
>   
I've tried with the last SVN.
It's working with the said file because the said file is simple (Y scan 
is index 0).
In your code you make the assumption the non-downsampled plane is with 
index 0, and there is only 1 component in the scan.
   (at line 1706, h = s->h_max / s->h_scount[0], then mb_width = width / 
(h * block_size) ) => this works because i will always be zero.
In my code, I take into account all possible cases (even not simple one 
where Y scan can be any index)
   (at line 1744, mb_width = width * h_count[index] / (h_max * 
block_size) ) => this works anywhere even when i is not zero

In your code, you also make (mb_width * mb_height * nb_blocks * 3 - 1) 
useless tests in decode_scan function
(look at all if() blocks in mjpeg_decode_scan function).
Again, you make the assumption the non-downsampled plane is with index 0
(that's why s->dc_index[i] (and all) works when nb_components is 1, in 
that case, and in that case only, i = 0 == index == id, and standard 
states that usually index != id)
Worst, you don't reset the right DC offset between scan decoding (you 
always reset the DC offset with index 0, it's working because the third 
parameter of decode_block is the index into the DC offset array, and it 
will always be 0 in the current code, while it should be id)
 
Anyway, Michael trust your code more than mine because it's "only 
removing 6 lines and changing 2" whatever the "non-standard" state of 
your decoding.

What I'm concluding, is that you prefer a "minimal dirty hack" to modify 
less possible code, even if you know that it will not work in the future 
(because it is not compliant) than making the "big" (my code adds 34 
real lines of debugged code, but will be faster because of only 1 test) 
change to be compliant to any possible JPEG.

>> Then, still don't believe me, apply my patch in a separate directory to 
>> the last SVN version, build, and try the same command line.
>> You'll get a good image as output. That's all.
>>
>> Then trust who you want, the kostya version works for progressive jpeg, 
>> not multiple scan jpegs, my version add multiple scan jpeg support
>> but as nothing to do with progressive jpeg (it doesn't break anything at 
>> all).
>>
>> Anyway, hope you don't take it as a personal attack, what I want is my 
>> stream correctly decoded.
>> If it's already done and working, it's great, whoever made it.
>> Currently it's not, so I've added it, I've provided all the materials to 
>> check if it's working.
>> I've corrected the patch so it is like the devs said it should be, so 
>> what next ?
>>
>> Regards,
>>
>> -- 
>> Cyril RUSSO
>>
>>
>>
>>     
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
>
>   


-- 
Cyril RUSSO