[FFmpeg-devel] [PATCH] make stream-switching work with MOV demuxer

Baptiste Coudurier baptiste.coudurier
Wed Jun 24 11:08:13 CEST 2009


Reimar D?ffinger wrote:
> On Wed, Jun 24, 2009 at 01:43:23AM -0700, Baptiste Coudurier wrote:
>> Reimar D?ffinger wrote:
>>> On Wed, Jun 24, 2009 at 12:50:20AM -0700, Baptiste Coudurier wrote:
>>>> Reimar D?ffinger wrote:
>>>>> On Tue, Jun 23, 2009 at 01:06:58PM -0700, Baptiste Coudurier wrote:
>>>>>> The other patch looks ok, and it is not uglier IMHO, it will allow sc =
>>>>>> msc removal since sc == st->priv_data, and sc->ffindex will no longer be
>>>>>> needed.
>>>>> Ok, let's get that part out of the way, see attached patch.
>>>>> Personally I'd even suggest making a
>>>>> AVIndexEntry *find_next_sample(AVFormatContext *s, AVStream **st);
>>>>> or similar that contains the whole for loop, making mov_read_packet
>>>>> a bit more self-documenting.
>>>> Yes good idea and patch ok.
>>> Not sure if the idea is that good, but here is a patch for that extra
>>> function, too.
>>> Names, as always, are of course open for improvements.
>> I like having functions names starting with mov_ or mp4_ in this file.
>> Except that if you want to apply it, I'm ok.
> 
> It makes the next patch slightly simpler (no need to reset sample).
> Attached is what remains of the patch you prefer.
> It's a bit questionable where exactly the st->discard checks should be
> placed, e.g. if the code setting pkt->stream_index or pkt->flags should
> be avoided with AVDISCARD_ALL or not. I placed it where it looked best
> to me, but I admit it is purely subjective.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: libavformat/mov.c
> ===================================================================
> --- libavformat/mov.c	(revision 19263)
> +++ libavformat/mov.c	(working copy)
> @@ -2063,7 +2063,7 @@
>      for (i = 0; i < s->nb_streams; i++) {
>          AVStream *avst = s->streams[i];
>          MOVStreamContext *msc = avst->priv_data;
> -        if (avst->discard != AVDISCARD_ALL && msc->pb && msc->current_sample < avst->nb_index_entries) {
> +        if (msc->pb && msc->current_sample < avst->nb_index_entries) {
>              AVIndexEntry *current_sample = &avst->index_entries[msc->current_sample];
>              int64_t dts = av_rescale(current_sample->timestamp, AV_TIME_BASE, msc->time_scale);
>              dprintf(s, "stream %d, sample %d, dts %"PRId64"\n", i, msc->current_sample, dts);
> @@ -2102,6 +2102,7 @@
>      sc = st->priv_data;
>      /* must be done just before reading, to avoid infinite loop on sample */
>      sc->current_sample++;
> +    if (st->discard != AVDISCARD_ALL) {
>      if (url_fseek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
>          av_log(mov->fc, AV_LOG_ERROR, "stream %d, offset 0x%"PRIx64": partial file\n",
>                 sc->ffindex, sample->pos);
> @@ -2120,6 +2121,7 @@
>              return ret;
>      }
>  #endif
> +    }
>      pkt->stream_index = sc->ffindex;
>      pkt->dts = sample->timestamp;
>      if (sc->ctts_data) {
> @@ -2139,6 +2141,7 @@
>          pkt->duration = next_dts - pkt->dts;
>          pkt->pts = pkt->dts;
>      }
> +    if (st->discard == AVDISCARD_ALL) goto retry;

goto at the line would look better IMHO and some empty lines in other
hunks would not be bad, this code needs some air it seems.

And it looks fine to me.

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org



More information about the ffmpeg-devel mailing list