Re: AVCHD/H.264 decoder: further development/corrections

Michael Niedermayer wrote:
> On Sun, Jan 25, 2009 at 08:08:06PM +0100, Ivan Schreter wrote:
> [...]
>   
>> Although the decoder itself takes this into account, the interface in 
>> libavformat doesn't. Thus, currently only video having full frames per 
>> packet decodes really correctly (and this also only with not-yet-applied 
>> patch concerning frame types). Reason: av_read_frame() doesn't return 
>> whole frames, although it is documented so.
>>     
>
> "decoding" of fields and even field/frame mixes works perfectly, and bitexact
> you can try the reference bitstreams ...
> what doesnt work is the timestamps and these cause the user apps o drop and
> duplicate "randomly"
>   
That's what I said. Decoder as such works correctly.

>   
>> *Alternative solution:* Return field packet from h264_parse() 
>> immediately, but somehow tell libavformat that the packet does not 
>> represent a full frame and second field has to be read as well. Read it 
>> in libavformat, extending the existing packet. Thus, av_read_frame() 
>> returns then full frame.
>>     
>
> you might want to look at
> svn di -r12162:12161
>   
I will. But the problem is, even if av_read_frame() returns proper 

Re: [PATCH] AVCHD/H.264 parser: determination of frame type, question about timestamps

Michael Niedermayer wrote:
> On Mon, Jan 26, 2009 at 08:42:17AM +0100, Ivan Schreter wrote:
> [...]
>   
>> We have a stream with pictures containing (T1/B1/T2==T1), (B2/T3/B3==B2) 
>> fields. That's two H.264 pictures, but 3 frames. Each av_read_frame() 
>> should return a packte containing exactly single frame. But we have just 
>> 2 packets, which need to be returned in 3 calls to av_read_frame(), 
>> according to API. Further, the DTS must be set correctly as well for the 
>> three AVPackets in order to get the timing correct. How do you want to 
>> handle this?
>>     
>
> i dont see where you get 3 calls of av_read_frame(),
> there are 2 or 4 access units not 3 unless one is coded as 2 fields
> and 1 is a frame
>
>   
No, we don't have 3 calls. First of all, I meant two pictures with 
SEI_PIC_STRUCT_TOP_BOTTOM_TOP and SEI_PIC_STRUCT_BOTTOM_TOP_BOTTOM. This 
generates 3 frames in the display. But the caller has to call 
av_read_frame() only twice, so he doesn't get all required timestamps. 
First decoded frame will have timestamp 1, second decoded frame 
timestamp 2 or 3, depending on how it's handled in H.264 decoder. One 
frame has to be added in between, with fields from both frames. This is 
currently not possible to express.

I suppose, there is the need to do something like repeat_pic on field 
level, but this means API change.

Re: [PATCH] give av_reduce function arguments more meaningful names

On Sun, Feb 01, 2009 at 12:33:41AM +0100, Michael Niedermayer wrote:
> On Sun, Feb 01, 2009 at 12:16:05AM +0100, Diego Biurrun wrote:
> > Calling the numerator 'num' instead of 'nom' makes more sense to me.
> 
> ok of course

Applied.

> also you likely should grep for nom i possibly made this mistake more
> often (i do remember correcting this one myself somewhere else)

I'll have a look around.

Diego

Re: [PATCH] RTSP-MS 13/15: transport context

On Sat, Jan 31, 2009 at 11:18:15AM -0500, Ronald S. Bultje wrote:
> Hi Michael,
> 
> 2009/1/10 Michael Niedermayer <michaelni <at> gmx.at>:
> > On Tue, Jan 06, 2009 at 08:49:55AM -0500, Ronald S. Bultje wrote:
> >> On Tue, Jan 6, 2009 at 12:17 AM, Ronald S. Bultje <rsbultje <at> gmail.com> wrote:
> >> > this patch adds a "parent class" to RTPDemuxContext and
> >> > RDTDemuxContext, called transport_context, which contains the only
> >> > shared structure between the two: the RTSP AVFormatContext. I need
> >> > that for my next patch to be able to retrieve the ASF demuxer's
> >> > AVFormatContext from the RTSPState (in the RTSP
> >> > AVFormatContext->priv_data) so I can call ASF demuxer functions in the
> >> > RTP payload parsing function.
> >
> > I dont understand why you need this, your explanation makes no sense,
> > thus i will not approve it.
> 
> Sorry for the bad explanation. The reason I need it is as follows:
> when reading an RTSP stream, the RTSP demuxer will determine whether
> this stream contains RDT or RTP packets (this is called the
> transport). We then call rtp_parse_packet() or ff_rdt_parse_packet()
> depending on the transport. Each of them have a special
> "transport-specific data" structure, RDTDemuxContext and
> RTPDemuxContext. We call functions with these contexts.
> 
> In the Transport are RTP/RDT data "payload" packets, and these are
> sort of orthogonal to the transport. RDT commonly contains Realmedia
> payload packets, whereas RTP can contain anything (ASF packets, MPEG-4
> audio/video, etc.). In theory, RDT can also stream any of these (and
> the Real servers support that). So, ff_rdt_parse_packet() or

Re: [PATCH] RTSP-MS 10/15: ASF header parsing

On Sun, Feb 01, 2009 at 12:36:52AM +0100, Michael Niedermayer wrote:
> On Sat, Jan 31, 2009 at 10:23:52AM -0500, Ronald S. Bultje wrote:
> > Hi Michael,
> > 
> > 2009/1/10 Michael Niedermayer <michaelni <at> gmx.at>:
> > > On Tue, Jan 06, 2009 at 08:49:06AM -0500, Ronald S. Bultje wrote:
> > >> +void ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p)
> > >> +{
> > >> +    if (av_strstart(p, "pgmpu:data:application/vnd.ms.wms-hdr.asfv1;base64,", &p)) {
> > >> +        ByteIOContext gb;
> > >> +        RTSPState *rt = s->priv_data;
> > >> +        int len = strlen(p) * 6 / 8;
> > >> +        char *buf = av_mallocz(len);
> > >> +        av_base64_decode(buf, p, len);
> > >> +
> > >> +        init_put_byte(&gb, buf, len, 0, NULL, NULL, NULL, NULL);
> > >> +        if (rt->asf_ctx) {
> > >> +            av_close_input_stream(rt->asf_ctx);
> > >> +            rt->asf_ctx = NULL;
> > >> +        }
> > >> +        av_open_input_stream(&rt->asf_ctx, &gb, "", &asf_demuxer, NULL);
> > >
> > >> +        rt->asf_gb_pos = url_ftell(&gb);
> > >
> > > unused
> > >
> > >> +        av_free(buf);
> > >
> > > at that point you have a ByteIOContext in your context with a deallocated
> > > buffer. This is ugly.

Re: [PATCH] RTSP-MS 13/15: transport context

Hi,

On Sat, Jan 31, 2009 at 7:28 PM, Michael Niedermayer <michaelni <at> gmx.at> wrote:
> I dont have a clear understanding or the RTP/RTSP code and so i wont
> approve patches to it except for the parts that i do understand.
> Your explanation above is lacking badly at many points, example being
> "In my parse_packet() implementation for the RTP/ASF payloads, I want
>  to use a value from the RTSP demuxer's RTSPState"
> which value? and why? the whole just smells even without understanding
> the whole picture it feels very hackish.

For ASF-over-RTP, I use the ASF demuxer for all ASF-related stuff. I
store the ASF demuxer context in the RTSP demuxer "private" context
(RTSPState)->asf_ctx, so that it's shared between all streams in the
session. That value needs to be accessible from the streams.

Ronald

Re: [PATCH] RTSP-MS 10/15: ASF header parsing

Hi,

On Sat, Jan 31, 2009 at 7:32 PM, Michael Niedermayer <michaelni <at> gmx.at> wrote:
> On Sun, Feb 01, 2009 at 12:36:52AM +0100, Michael Niedermayer wrote:
>> On Sat, Jan 31, 2009 at 10:23:52AM -0500, Ronald S. Bultje wrote:
>> > Do you want me to set s->gb = NULL; or would you prefer an alternative solution?
>>
>> AVFormatContext has no member baned gb, what are you trying to say?
>
> s/baned/named/ it seems my finger was one key to far to the left :)

s/gb/pb/, sorry for the typo.

Ronald

Re: [PATCH] AVCHD/H.264 parser: determination of frame type, question about timestamps

On Sun, Feb 01, 2009 at 01:17:24AM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
> > On Mon, Jan 26, 2009 at 08:42:17AM +0100, Ivan Schreter wrote:
> > [...]
> >   
> >> We have a stream with pictures containing (T1/B1/T2==T1), (B2/T3/B3==B2) 
> >> fields. That's two H.264 pictures, but 3 frames. Each av_read_frame() 
> >> should return a packte containing exactly single frame. But we have just 
> >> 2 packets, which need to be returned in 3 calls to av_read_frame(), 
> >> according to API. Further, the DTS must be set correctly as well for the 
> >> three AVPackets in order to get the timing correct. How do you want to 
> >> handle this?
> >>     
> >
> > i dont see where you get 3 calls of av_read_frame(),
> > there are 2 or 4 access units not 3 unless one is coded as 2 fields
> > and 1 is a frame
> >
> >   
> No, we don't have 3 calls. First of all, I meant two pictures with 
> SEI_PIC_STRUCT_TOP_BOTTOM_TOP and SEI_PIC_STRUCT_BOTTOM_TOP_BOTTOM. 

> This 
> generates 3 frames in the display. 

no, it generates 2 frames each with a duration of 1.5 each

> But the caller has to call 
> av_read_frame() only twice, so he doesn't get all required timestamps. 
> First decoded frame will have timestamp 1, second decoded frame 

Re: [PATCH] RTSP-MS 10/15: ASF header parsing

On Sat, Jan 31, 2009 at 07:37:55PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Sat, Jan 31, 2009 at 7:32 PM, Michael Niedermayer <michaelni <at> gmx.at> wrote:
> > On Sun, Feb 01, 2009 at 12:36:52AM +0100, Michael Niedermayer wrote:
> >> On Sat, Jan 31, 2009 at 10:23:52AM -0500, Ronald S. Bultje wrote:
> >> > Do you want me to set s->gb = NULL; or would you prefer an alternative solution?
> >>
> >> AVFormatContext has no member baned gb, what are you trying to say?
> >
> > s/baned/named/ it seems my finger was one key to far to the left :)
> 
> s/gb/pb/, sorry for the typo.

asf_gb_pos is unused as well

and setting pb to NULL is of course better than not doing it.

[...]

--

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu

Re: [PATCH] av_metadata_count()

On Fri, 30 Jan 2009 17:33:02 +0100
Michael Niedermayer <michaelni <at> gmx.at> wrote:

> On Fri, Jan 30, 2009 at 01:43:34AM +0100, Aurelien Jacobs wrote:
> > Hi,
> > 
> > Attached patch adds a new metadata API func to get the number of
> > available tags among a list of keys. This func will be useful
> > at least for the following muxers: asf, mov, mp3.
> > It can also be quite handy for a user application.
> 
> i cant see this to be that usefull, please first show the code
> that would benefit from this.

OK. As an example, here is a snipet from movenc:
    // only save meta tag if required
    if (s->title[0] || s->author[0] || s->album[0] || s->year ||
        s->comment[0] || s->genre[0] || s->track) {
It could be replaced by something very ugly:
    // only save meta tag if required
    if (av_metadata_get(s->metadata, "title"  , NULL, 0) ||
        av_metadata_get(s->metadata, "author" , NULL, 0) ||
        av_metadata_get(s->metadata, "album"  , NULL, 0) ||
        av_metadata_get(s->metadata, "year"   , NULL, 0) ||
        av_metadata_get(s->metadata, "comment", NULL, 0) ||
        av_metadata_get(s->metadata, "genre"  , NULL, 0) ||
        av_metadata_get(s->metadata, "track"  , NULL, 0)) {
Or by something simpler like:
    static const char const *mov_tags[] = {
        "title", "author", "album", "year", "copyright", "comment",