[03/12] RFC: movenc: Try supporting flushing the muxer by doing av_write_frame(..., NULL);

diff mbox

---
 libavformat/movenc.c |   10 ++++++++--
 libavformat/utils.c  |    7 ++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

On 01/20/2012 08:20 AM, Martin Storsjö wrote:

> ---
>  libavformat/movenc.c |   10 ++++++++--
>  libavformat/utils.c  |    7 ++++++-
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index f497b6a..06ad908 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -2370,13 +2370,19 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>  {
>      MOVMuxContext *mov = s->priv_data;
>      AVIOContext *pb = s->pb;
> -    MOVTrack *trk = &mov->tracks[pkt->stream_index];
> +    MOVTrack *trk = &mov->tracks[pkt ? pkt->stream_index : 0];
>      AVCodecContext *enc = trk->enc;
>      unsigned int samplesInChunk = 0;
> -    int size= pkt->size;
> +    int size= pkt ? pkt->size : 0;
>      uint8_t *reformatted_data = NULL;
>  
>      if (!s->pb->seekable) return 0; /* Can't handle that */
> +
> +    if (!pkt) {
> +        mov_flush_fragment(s);
> +        return 0;
> +    }
> +
>      if (!size) return 0; /* Discard 0 sized packets */
>  
>      if ((mov->max_fragment_duration && trk->entry &&
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 373f068..2dbe9a5 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3118,7 +3118,12 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt){
>  
>  int av_write_frame(AVFormatContext *s, AVPacket *pkt)
>  {
> -    int ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
> +    int ret;
> +
> +    if (!pkt)
> +        return s->oformat->write_packet(s, pkt);
> +
> +    ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
>  
>      if(ret<0 && !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
>          return ret;


While a nice idea, I think we need to consider that other muxers may
fail when getting a NULL packet, and we shouldn't require the user to do
special-casing for mov.  I think the options here are to add a format
flag for this similar to CODEC_CAP_DELAY in lavc, or we go through all
the muxers and make sure they can also handle this.

-Justin

On Fri, 20 Jan 2012, Justin Ruggles wrote:

> On 01/20/2012 08:20 AM, Martin Storsjö wrote:
>
>> ---
>>  libavformat/movenc.c |   10 ++++++++--
>>  libavformat/utils.c  |    7 ++++++-
>>  2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 373f068..2dbe9a5 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -3118,7 +3118,12 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt){
>>
>>  int av_write_frame(AVFormatContext *s, AVPacket *pkt)
>>  {
>> -    int ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
>> +    int ret;
>> +
>> +    if (!pkt)
>> +        return s->oformat->write_packet(s, pkt);
>> +
>> +    ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
>>
>>      if(ret<0 && !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
>>          return ret;
>
>
> While a nice idea, I think we need to consider that other muxers may
> fail when getting a NULL packet, and we shouldn't require the user to do
> special-casing for mov.  I think the options here are to add a format
> flag for this similar to CODEC_CAP_DELAY in lavc, or we go through all
> the muxers and make sure they can also handle this.

Hmm, like AVFMT_ALLOW_FLUSH, and only passing the NULL packet through to 
the muxer in that case? That is doable, and allows implementing this for 
this muxer at the moment without having to update every single other one 
at the same time.

As a side note, currently you can't do av_write_frame(..., NULL) at all, 
it will crash, so the current patch shouldn't change anything for existing 
users, except that they could do this call if they know the muxer can 
handle it. But that's of course not a nice API.

// Martin

On 01/20/2012 10:38 AM, Martin Storsjö wrote:

> On Fri, 20 Jan 2012, Justin Ruggles wrote:
> 
>> On 01/20/2012 08:20 AM, Martin Storsjö wrote:
>>
>>> ---
>>>  libavformat/movenc.c |   10 ++++++++--
>>>  libavformat/utils.c  |    7 ++++++-
>>>  2 files changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>> index 373f068..2dbe9a5 100644
>>> --- a/libavformat/utils.c
>>> +++ b/libavformat/utils.c
>>> @@ -3118,7 +3118,12 @@ static int compute_pkt_fields2(AVFormatContext *s, AVStream *st, AVPacket *pkt){
>>>
>>>  int av_write_frame(AVFormatContext *s, AVPacket *pkt)
>>>  {
>>> -    int ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
>>> +    int ret;
>>> +
>>> +    if (!pkt)
>>> +        return s->oformat->write_packet(s, pkt);
>>> +
>>> +    ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
>>>
>>>      if(ret<0 && !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
>>>          return ret;
>>
>>
>> While a nice idea, I think we need to consider that other muxers may
>> fail when getting a NULL packet, and we shouldn't require the user to do
>> special-casing for mov.  I think the options here are to add a format
>> flag for this similar to CODEC_CAP_DELAY in lavc, or we go through all
>> the muxers and make sure they can also handle this.
> 
> Hmm, like AVFMT_ALLOW_FLUSH, and only passing the NULL packet through to 
> the muxer in that case? That is doable, and allows implementing this for 
> this muxer at the moment without having to update every single other one 
> at the same time.
> 
> As a side note, currently you can't do av_write_frame(..., NULL) at all, 
> it will crash, so the current patch shouldn't change anything for existing 
> users, except that they could do this call if they know the muxer can 
> handle it. But that's of course not a nice API.


Yes, and AVFMT_ALLOW_FLUSH sounds fine to me.

-Justin