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

[Mats Peterson]

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?

Mats