[Ffmpeg-devel] Re: [PATCH] PKT_FLAG_B

[Baptiste Coudurier]

Hi

Michael Niedermayer wrote:
> [...]
> 
> no it depends on the container, mpeg in avi, nut, mov, ... will not
> be parsed
> 
>> Also assuming b frame regarding pts/dts difference is not exact, 
> 
> elaborate / bugreport
> and i prefer to fix a bug instead of changing the AVPacket or AVParser
> API to workaround it
> 

Im sending a sample, Atm test is wrong for containers that does not have
pts (avi), and anyway genpts does not generate correct pts for streams
with B frames in avi. Parser detects correctly that frame is a B one.

Let's see:
ffmpeg -i <file.mpg> -vcodec h264 -bf 2 test.avi
ffmpeg -i test.avi -vcodec copy test.mp4. Plays but ctts:
count 1 duration 0
count 1 duration -1
count 1 duration -2
count 1 duration -3
count 1 duration -4
count 1 duration -5
...
count 1 duration -36

ffmpeg -genpts -i test.avi -vcodec copy test.mp4. Plays but ctts:
count 36 duration 1
count 1 duration -36

>> and
>> has_b_frames is not set when stream copying.
> 
> the parser should set it, if it does not thats a bug which you can fix if
> you like, alternatively you can assume it to be always 1
> 

Parser depends on container, no ? Should demuxer set has_b_frame ?
What about always invoking parser when stream copying ?

Well assuming oftenly lead me to problems when reviewing, and when
interpreting, so I would better stop assuming anything.

IMHO If has_b_frames is 0 then stream does not contain b frames.

>> [...]
>>
>> Players (broadcast servers) does not behave correctly if B and P values
>> are 0, random will be just wrong.
> 
> elaborate, which broadcast server does what? the spec clearly says
> "These values are provided for information purposes only."

Grass Valley M Series. During import it will simply reject files with B
and P > 0 since M Series cannot decode them for example. Unfortunately
broadcast servers are not as robust as our beloved ffmpeg/mplayer and
almost always rely on formats flags/infos/metadatas whatever.

If you fake the number, it will be ingested and will crash the server
when trying to play. Same goes for Closed gop flag btw. That's the next
thing I need to figure out and detect.

> and about setting them correctly for the stream copy case, thats not
> possible as they represent encoder parameters not parameters which depend
> on the stream

Well UMF might be for information purposes only, but Track description
MPEG auxilliary data defines the stream, it should be possible to set
those informations.

>>> and the media field number should be set to dts and time line field
>>> number to pts, not a random +=2 every frame variable
>> That is right. media field number is actually dts, 
> 
> good so we agree
> 
> 
>> computed correctly
>> internally 
> 
> no your calculation is wrong, try a telecined mpeg 

Humm you mean MPEG 1/2 field stored right ? If so then yes it is wrong.

Current code (wrong for field stored mpeg, but only that corner case)
works on all broadcast servers I tried on, and is robust against dts
errors, while relying on pkt dts/pts is far less reliable IMHO.

Of course it would be perfect if dts/pts handling code was 100%
reliable. Still when I think about the mess between formats, I really
feel lost sometimes.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312