[FFmpeg-devel] [PATCH 1/3] lavf/riffenc: Improve spec compliance

Sat Mar 12 11:30:56 CET 2016

On 03/12/2016 10:58 AM, Michael Niedermayer wrote:
> On Fri, Mar 11, 2016 at 10:26:43PM +0100, Mats Peterson wrote:
>> On 03/11/2016 06:45 PM, Michael Niedermayer wrote:
>>> On Fri, Mar 11, 2016 at 03:31:55PM +0100, Mats Peterson wrote:
>>>> On 03/11/2016 03:27 PM, Mats Peterson wrote:
>>>>> On 03/11/2016 03:21 PM, Mats Peterson wrote:
>>>>>> On 03/11/2016 03:13 PM, Mats Peterson wrote:
>>>>>>> Michael Niedermayer <michael at niedermayer.cc> skrev: (11 mars 2016
>>>>>>> 15:05:49 CET)
>>>>>>>> On Fri, Mar 11, 2016 at 02:12:56PM +0100, Mats Peterson wrote:
>>>>>>>>> Mats Peterson <matsp888-at-yahoo.com at ffmpeg.org> skrev: (11 mars 2016
>>>>>>>> 14:06:19 CET)
>>>>>>>>>> Mats Peterson <matsp888-at-yahoo.com at ffmpeg.org> skrev: (11 mars
>>>>>>>> 2016
>>>>>>>>>> 13:55:20 CET)
>>>>>>>>>>> Michael Niedermayer <michael at niedermayer.cc> skrev: (11 mars 2016
>>>>>>>>>>> 13:49:32 CET)
>>>>>>>>>>>> On Fri, Mar 11, 2016 at 01:28:47PM +0100, Mats Peterson wrote:
>>>>>>>>>>>>> On 03/11/2016 01:25 PM, Mats Peterson wrote:
>>>>>>>>>>>>>> On 03/11/2016 01:14 PM, Michael Niedermayer wrote:
>>>>>>>>>>>>>>> On Fri, Mar 11, 2016 at 05:17:18AM +0100, Mats Peterson wrote:
>>>>>>>>>>>>>>>> On 03/11/2016 05:10 AM, Mats Peterson wrote:
>>>>>>>>>>>>>>>>> Forget patch 2/3 and 3/3 from the old patch set.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>  From the Microsoft documentation for BITMAPINFOHEADER at
>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/dd318229%28v=vs.85%29.aspx:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> "biSize: Specifies the number of bytes required by the
>>>>>>>>>>> structure.
>>>>>>>>>>>> This
>>>>>>>>>>>>>>>>> value does not include the size of the color table or the
>>>>>>>> size
>>>>>>>>>>> of
>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> color masks, if they are appended to the end of structure."
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> So, biSize is always 40. Also, Windows Media Player won't
>>>>>>>>>> detect
>>>>>>>>>>>> video
>>>>>>>>>>>>>>>>> encoded with Microsoft Video 1 in 8 bpp mode if this value
>>>>>>>> is
>>>>>>>>>>>> anything
>>>>>>>>>>>>>>>>> else than 40. I don't know about other codecs, they probably
>>>>>>>>>>>> work.
>>>>>>>>>>>>>>>>> Anyway, we should stick with the specs, and not include the
>>>>>>>>>>>> palette
>>>>>>>>>>>>>>>>> size
>>>>>>>>>>>>>>>>> in that field.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Regarding the biClrUsed field, I'm setting it to 1 <<
>>>>>>>>>>>>>>>>> bits_per_coded_sample if palettized video, since setting it
>>>>>>>> to
>>>>>>>>>> 0
>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>> another case where it won't work with Windows Media Player
>>>>>>>> and
>>>>>>>>>>>>>>>>> Microsoft
>>>>>>>>>>>>>>>>> Video 1 in 8 bpp mode.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Mats
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Once, again, HuffYUV has its own variant of BITMAPINFOHEADER
>>>>>>>>>> that
>>>>>>>>>>>>>>>> *does* include the size of the Huffman tables in biSize.
>>>>>>>> That's
>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> only exception as far as I know.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> is huffyuv really the exception ? and not raw rgb or palettes
>>>>>>>> ?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> asv1 and asv2 do not have 40 there
>>>>>>>>>>>>
>>>>>>>>>>>>>>> which codec with a non-palette global header puts 40 there ?
>>>>>>>>>>>>
>>>>>>>>>>>> [...]
>>>>>>>>>>>>
>>>>>>>>>>>>> As I said before, Microsoft Video 1 in 8-bit mode and RLE4/RLE8
>>>>>>>>>>>>> won't even display in Windows Media Player if this value is
>>>>>>>>>> anything
>>>>>>>>>>>>> else than 40.
>>>>>>>>>>>>
>>>>>>>>>>>> msv1 / RLE4/8 doesnt have a "Non palette global header"
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> [...]
>>>>>>>>>>>
>>>>>>>>>>> That's correct. Sorry. Anyway, the only case I know of is HuffYUV
>>>>>>>> with
>>>>>>>>>>> its special variant of BITMAPINFOHEADER that includes the Huffman
>>>>>>>>>>> tables. Is there stated anywhere in the ASUS specs that the size of
>>>>>>>>>> the
>>>>>>>>>>> extra data following the BITMAPINFOHEADER should be included in
>>>>>>>>>> biSize?
>>>>>>>>>>> If not, it should be 40.
>>>>>>>>>>>
>>>>>>>>>>> Mats
>>>>>>>>>>> --
>>>>>>>>>>> Mats Peterson
>>>>>>>>>>> http://matsp888.no-ip.org/~mats/
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> ffmpeg-devel mailing list
>>>>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>>>>
>>>>>>>>>> Those asv1/asv2 files play just fine with a biSize of 40, at that.
>>>>>>>>>>
>>>>>>>>>> Mats
>>>>>>>>>> --
>>>>>>>>>> Mats Peterson
>>>>>>>>>> http://matsp888.no-ip.org/~mats/
>>>>>>>>>> _______________________________________________
>>>>>>>>>> ffmpeg-devel mailing list
>>>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>>>
>>>>>>>>> Should we perhaps "tighten" the "specs" (I saw that you have written
>>>>>>>> some documentation on asv1/asv2 in FFmpeg), and include those extra
>>>>>>>> bytes in biSize? I suppose this is somewhat of a proprietary FFmpeg
>>>>>>>> invention.
>>>>>>>>
>>>>>>>> we didnt invent ASUS video 1 and 2 nor huffyuv
>>>>>>>>
>>>>>>>> which codec with a non-palette global header puts 40 there ?
>>>>>>>>
>>>>>>>> the ms specs just says the color table & mask isnt part of biSize
>>>>>>>> it doesnt say the global header isnt, or iam looking at the wrong text
>>>>>>>> or location in the text/spec
>>>>>>>>
>>>>>>>> [...]
>>>>>>>
>>>>>>> Again, only one I know of is HuffYUV, and the specs define a custom
>>>>>>> BITMAPINFOHEADER that includes the Huffman tables. The "global data"
>>>>>>> in ASUS is only 8 bytes. Do they regard the BITMAPINFOHEADER as being
>>>>>>> part of this global data as well? And should we? A BITMAPINFOHEADER is
>>>>>>> a BITMAPINFOHEADER is a BITMAPINFOHEADER...
>>>>>>>
>>>>>>> Mats
>>>>>>>
>>>>>>
>>>>>> Do you have a sample file with asv1/asv2 *created by ASUS* where biSize
>>>>>> isn't 40?
>>>>>>
>>>>>> Mts
>>>>>>
>>>>>
>>>>> I got the asv1/asv2 samples from the MPlayer sample directory. Are they
>>>> >from ASUS? They all use a biSize of 48.
>>>>>
>>>>> Mats
>>>>>
>>>>
>>>> Nope. Written with libavformat.
>>>
>>> why do you think they are written by libavformat ?
>>>
>>> i see:
>>> "C:\PROGRAM FILES\ASUS\ASUS LIVE\ASUSLIVE.EXE -AVICAP32- ASUS Video Capture Driver, Version:  3.8.2.2"
>>>
>>> in asv2_320x240_3.avi
>>>
>>>
>>
>> Yes, you are right. I was looking at some other files. Sorry. So how
>> do we proceed from here? Should we make an exception to the "40-byte
>> rule" for HuffYUV, ffvhuff, asv1 and asv2 in the meantime?
>
> before writing a system based on a list of exceptions, please awnser
> the question
>
> which codec with a non-palette global header puts 40 there

In my latest 2-part patch set I've included asv1 and asv2 in that 
"list", but I'm still ambivalent about it. Here's why:

Any codec that doesn't specify a "custom" BITMAPINFOHEADER like HuffYUV 
(which is already breaking the specs) should use a biSize of 40.

The HuffYUV specs at http://multimedia.cx/huffyuv.txt includes the 
Huffman tables and some other stuff inside its own "home-brew" 
BITMAPINFOHEADER. This is already breaking the specs, using a "custom" 
BITMAPINFOHEADER, but I've accepted the facts in that single case. For 
asv1/asv2 there is no mention anywhere that biSize should include the 8 
extra bytes of the so called "global header". And it shouldn't for any 
other extradata in any other codec either, unless the specs for the 
codec explicitly state it (and that would break the specs).

Microsoft explicitly says that biSize is "The number of bytes required 
by the structure.", and the structure is the very BITMAPINFOHEADER, no 
less, no more. Hence, biSize should ALWAYS be 40 if you ask me, 
regardless of any palette or other proprietary non-palette data 
following it. HuffYUV is the one exception since it defines its own 
BITMAPINFOHEADER variant (unfortunately).

It's easy to calculate the size of this extra data by just subtracting 
the size of the BITMAPINFOHEADER (40) from the size of the strf chunk. 
And AVI files using any of the aforementioned codecs demux just fine 
with a biSize of 40, including HuffYUV.

Mats