[FFmpeg-devel] [PATCH v3] avformat/rtpdec_rfc4175: Fix incorrect copy_offset calculation

[Michael Niedermayer]

On Tue, Aug 06, 2019 at 05:42:22AM +0000, Jacob Siddall wrote:
> > On Mon, Aug 05, 2019 at 06:05:29PM +0100, Kieran Kunhya wrote:
> > > On Mon, 5 Aug 2019 at 17:10, Michael Niedermayer <michael at niedermayer.cc>
> > > wrote:
> > >
> > > > On Thu, Jun 27, 2019 at 06:06:22AM +0000, Jacob Siddall wrote:
> > > > > The previous calculation code did not account for the fact that the
> > > > > copy_offset for the start of the frame array is at index 0, yet the
> > > > > scan line number from the rfc4175 RTP header starts at 1.
> > > > > This caused 2 issues to appear:
> > > > > - The first scan line was being copied into the array where the second
> > > > >   scan line should be. This caused the resulting video to have a green
> > > > >   line at the top of it.
> > > > > - Since the packet containing the last scan line would fail the
> > > > >   calculation, the packet with the RTP marker would not be processed
> > > > >   which caused a log message saying "Missed previous RTP marker" to be
> > > > >   outputted for each frame.
> > > > >
> > > > > Signed-off-by: Jacob Siddall <kobe at live.com.au>
> > > > > ---
> > > > > Changes in v2:
> > > > >   - Don't handle packet if the line number is less than 1
> > > > >
> > > > > Section 12 in the VSF technical recommendation TR-03 specifies that the
> > > > > video scan line numbers should start at 1.
> > > > >
> > > > http://www.videoservicesforum.org/download/technical_recommendations/VSF_TR-03_2015-11-12.pdf
> > > >
> > > >
> > > SMPTE 2110-20 says line numbers start at 0. (Yes there are 3 different
> > > "standards" for handling the line number because of the geniuses in the
> > > broadcast industry).
> > 
> > Jacob, can you look into this ?
> > 
> > should i revert until this is fixed?
> > 
> > Thanks
> 
> Michael, yes lets revert this commit for now and come up with a better
> solution that can cater for both starting line number cases.

ok, will revert 


> 
> I think we could handle this in 1 of 2 ways:
> 1. Add some sort of flag which allows overriding of the value which line
>    numbers start from.
>    We could leave the default as line numbering starting from 0 (how the
>    previous code worked). If the flag is set, then the line numbering 
>    starts from 1.
> 2. Add code which detects what the value of the first line number is and
>    use that value for any future processing. This could work by assuming
>    line numbering starts at 1 until it sees a frame with line 0. This 
>    value would reside in  memory so if ffmpeg is restarted, the line 
>    number detection would re-run.
> 
> What are your thoughts on this?

It should be automatic, not requiring user input of course.
So if the format can be detected somehow thats best
Next best simply detecting if the first line is 0 or 1 and keeping that in
the nearest instance context probably.
Any global variables to hold that state would of course not work as
there can be multiple instances running at the same time

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190806/b21385f6/attachment.sig>