[FFmpeg-devel,05/10] lavf/dashenc: don't call flush_init_segment before avformat_write_header

Submitted by Rodger Combs on March 14, 2018, 6:24 a.m.

Details

Message ID 20180314062445.89909-5-rodger.combs@gmail.com
State New
Headers show

Commit Message

Rodger Combs March 14, 2018, 6:24 a.m.
Fixes crash when muxing MKV-in-DASH
---
 libavformat/dashenc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

kjeyapal@akamai.com March 18, 2018, 7:16 a.m.
On 3/14/18 11:54 AM, Rodger Combs wrote:
> Fixes crash when muxing MKV-in-DASH

> ---

>  libavformat/dashenc.c | 10 +++-------

>  1 file changed, 3 insertions(+), 7 deletions(-)

>

> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c

> index 5689aef811..63ff827583 100644

> --- a/libavformat/dashenc.c

> +++ b/libavformat/dashenc.c

> @@ -985,13 +985,6 @@ static int dash_init(AVFormatContext *s)

>  

>          av_log(s, AV_LOG_VERBOSE, "Representation %d init segment will be written to: %s\n", i, filename);

>  

> -        // Flush init segment

> -        // except for mp4, since delay_moov is set and the init segment

> -        // is then flushed after the first packets

> -        if (strcmp(os->format_name, "mp4")) {

> -            flush_init_segment(s, os);

> -        }

> -

>          s->streams[i]->time_base = st->time_base;

>          // If the muxer wants to shift timestamps, request to have them shifted

>          // already before being handed to this muxer, so we don't have mismatches

> @@ -1032,6 +1025,9 @@ static int dash_write_header(AVFormatContext *s)

>          OutputStream *os = &c->streams[i];

>          if ((ret = avformat_write_header(os->ctx, NULL)) < 0)

>              return ret;

> +

> +        if ((ret = flush_init_segment(s, os)) < 0)

> +            return ret;

I am fine with moving this here. But you have removed the strcmp for mp4 files.
Have you analyzed the impact of this change for mp4 files. Looks like something might break for mp4 files.
I would be more comfortable, if the strcmp and its related comment is also moved here.
>      }

>      ret = write_manifest(s, 0);

>      if (!ret)
Rodger Combs March 18, 2018, 7:21 a.m.
> On Mar 18, 2018, at 02:16, Jeyapal, Karthick <kjeyapal@akamai.com> wrote:
> 
> 
> 
> On 3/14/18 11:54 AM, Rodger Combs wrote:
>> Fixes crash when muxing MKV-in-DASH
>> ---
>> libavformat/dashenc.c | 10 +++-------
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>> 
>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>> index 5689aef811..63ff827583 100644
>> --- a/libavformat/dashenc.c
>> +++ b/libavformat/dashenc.c
>> @@ -985,13 +985,6 @@ static int dash_init(AVFormatContext *s)
>> 
>>         av_log(s, AV_LOG_VERBOSE, "Representation %d init segment will be written to: %s\n", i, filename);
>> 
>> -        // Flush init segment
>> -        // except for mp4, since delay_moov is set and the init segment
>> -        // is then flushed after the first packets
>> -        if (strcmp(os->format_name, "mp4")) {
>> -            flush_init_segment(s, os);
>> -        }
>> -
>>         s->streams[i]->time_base = st->time_base;
>>         // If the muxer wants to shift timestamps, request to have them shifted
>>         // already before being handed to this muxer, so we don't have mismatches
>> @@ -1032,6 +1025,9 @@ static int dash_write_header(AVFormatContext *s)
>>         OutputStream *os = &c->streams[i];
>>         if ((ret = avformat_write_header(os->ctx, NULL)) < 0)
>>             return ret;
>> +
>> +        if ((ret = flush_init_segment(s, os)) < 0)
>> +            return ret;
> I am fine with moving this here. But you have removed the strcmp for mp4 files.
> Have you analyzed the impact of this change for mp4 files. Looks like something might break for mp4 files.
> I would be more comfortable, if the strcmp and its related comment is also moved here.

After some more testing, I think you're correct. I'll resend with it moved.

>>     }
>>     ret = write_manifest(s, 0);
>>     if (!ret)
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>

Patch hide | download patch | download mbox

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 5689aef811..63ff827583 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -985,13 +985,6 @@  static int dash_init(AVFormatContext *s)
 
         av_log(s, AV_LOG_VERBOSE, "Representation %d init segment will be written to: %s\n", i, filename);
 
-        // Flush init segment
-        // except for mp4, since delay_moov is set and the init segment
-        // is then flushed after the first packets
-        if (strcmp(os->format_name, "mp4")) {
-            flush_init_segment(s, os);
-        }
-
         s->streams[i]->time_base = st->time_base;
         // If the muxer wants to shift timestamps, request to have them shifted
         // already before being handed to this muxer, so we don't have mismatches
@@ -1032,6 +1025,9 @@  static int dash_write_header(AVFormatContext *s)
         OutputStream *os = &c->streams[i];
         if ((ret = avformat_write_header(os->ctx, NULL)) < 0)
             return ret;
+
+        if ((ret = flush_init_segment(s, os)) < 0)
+            return ret;
     }
     ret = write_manifest(s, 0);
     if (!ret)