[FFmpeg-devel] [PATCH] matroska : Fix writing packets to a non-seekable AVIOContext

[Michael Niedermayer]

On Wed, Mar 07, 2012 at 03:47:43PM -0800, Aaron Colwell wrote:
> On Tue, Mar 6, 2012 at 10:24 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
> 
> >
> > I think the problem is that ffserver produces avio_tell() == 0 values
> > and that causes
> > "if (mkv->cluster_pos &&"
> > to fail
> > but iam guessing here a bit from your description of what the problem
> > is so i could be wrong
> >
> > If that is correct then to fix it, i think the best would be either to
> > make ffserver produce normal advancing url_tell() values
> > or
> > To change the default "not set" value of cluster_pos to something else
> > than 0 and change all the checks as well.
> >
> 
> Yes you are correct on both counts. Thanks for pointing me in the right
> direction. The new patch below fixes both problems.
> 
> ffserver appears to use a new dyn_packet_buf each time it calls
> av_write_frame() and was not setting the pos member variable to reflect the
> current stream position. I've added a one line change to fix that.
> 
> I also changed the code in matroskaenc.c to use -1 as the "I'm not
> currently working on a cluster" signal instead of 0. This avoids problems
> where avio_tell() returns 0. I've updated all the checks against
> cluster_pos and added a minor fix to mkv_add_cuepoint() to prevent invalid
> cue entries from getting created with invalid cluster_pos values.
> 
> Aaron
> 
> ---
>  ffserver.c                |    1 +
>  libavformat/matroskaenc.c |   11 ++++++-----
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/ffserver.c b/ffserver.c
> index bddcb7d..2b819d0 100644
> --- a/ffserver.c
> +++ b/ffserver.c
> @@ -2380,6 +2380,7 @@ static int http_prepare_data(HTTPContext *c)
>                          ret = ffio_open_dyn_packet_buf(&ctx->pb,
> max_packet_size);
>                      } else {
>                          ret = avio_open_dyn_buf(&ctx->pb);
> +                        ctx->pb->pos = c->data_count;
>                      }
>                      if (ret < 0) {
>                          /* XXX: potential leak */

doesnt this break when some muxer calls avio_seek() that cannot be
satisfied within the buffer ?



> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 0b36725..400ac97 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -372,7 +372,7 @@ static int mkv_add_cuepoint(mkv_cues *cues, int stream,
> int64_t ts, int64_t clus
>      if (entries == NULL)
>          return AVERROR(ENOMEM);
> 
> -    if (ts < 0)
> +    if (ts < 0 || cluster_pos < cues->segment_offset)
>          return 0;
> 
>      entries[cues->num_entries  ].pts = ts;

why is this needed ?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120309/8ab90d27/attachment.asc>