[FFmpeg-devel] [PATCH] Sync the NTP timestamps for RTP streams

[Martin Storsjö]

(Changed subject, since this discussion mainly was on this topic...)

On Thu, 11 Mar 2010, Michael Niedermayer wrote:

> On Wed, Mar 10, 2010 at 10:35:09PM +0200, Martin Storsj? wrote:
> > On Tue, 9 Mar 2010, Michael Niedermayer wrote:
> > 
> > > I assume you know how AVOptions are used to set things in AVFormatContext
> > > the idea is to do the same with AVFormatContext->priv_data
> > 
> > Ah, I see. That generally feels like a good idea. However, here's the list 
> > of open questions in this approach, as far as I can tell from the code:
> > 
> > Should one be able to do av_set_string3() directly on an AVFormatContext, 
> > with the name of a private context option, and this should either succeed 
> > or fail depending on which (de)muxer it actually is? In that case, 
> > av_find_opt would need to be extended to look for options other than just 
> > the list found in the AVClass.
> 
> this would be nice but not essential in an initial implementation
> Maybe a offset in the AVClass struct could point to priv_data
> or NULL for structs not having a priv_data with its own AVClass
> 
> 
> > 
> > Or if we're supposed to do the av_set_string3() on 
> > AVFormatContext->priv_data instead, we would have to make sure all 
> > instances of priv_data actually has an AVClass.
> > 
> > In ffmpeg.c/cmdutils.c, the options are temporarily stored in an 
> > AVFormatContext before they're later applied to the actual muxer using 
> > set_context_opts. In this scenario, setting a muxer-specific setting 
> > wouldn't work since the temporary AVFormatContext wouldn't have the 
> > private data for that particular muxer.
> 
> hmm, that may need a little redesign then
> i never liked that storing of things in that temporary contexts
> 
> either way codec specific options would make alot of people happy
> i think, personally i never understood it but it seems people really
> dont like adding their stuff to a ever growing context

If adding another field to the context is ok with you, the attached 
series would be another way of solving it.

Instead of storing NTP time (which isn't all that common outside of RTP), 
the field stores unix time (in microseconds). Also, I chose to store this 
in AVStream instead of in AVFormatContext, since it could be used to store 
extra info for the RTP streams when using the RTSP demuxer.

Is this acceptable, and does it seem generally usable for other things 
than this one in particular, so that it could be accepted in the general 
context?

// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-a-field-to-AVStream-for-specifying-a-real-world-.patch
Type: text/x-diff
Size: 960 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100311/eaeafeb3/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Move-the-NTP-offset-definitions-to-internal.h.patch
Type: text/x-diff
Size: 1274 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100311/eaeafeb3/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Use-AVStream-start_time_realtime-in-the-RTP-muxer.patch
Type: text/x-diff
Size: 1004 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100311/eaeafeb3/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Set-the-start_time_realtime-field-for-output-RTP-mux.patch
Type: text/x-diff
Size: 2304 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100311/eaeafeb3/attachment-0003.patch>