diff mbox

[FFmpeg-devel,v1] avformat/tee.c: steal bsf option before passing to fifo muxer

Message ID 20191006033727.8199-1-junli1026@gmail.com
State Superseded
Headers show

Commit Message

Jun Li Oct. 6, 2019, 3:37 a.m. UTC
Fix #7620
In the case tee muxer with both "bsf" and "use_fifo" parameters
wil trigger this bug. Tee muxer will first steal parameters (like "f",
"select"...) and then "use_fifo" will try reading out remaining options
and pass them to fifo as option "format_options".
Current code miss the part of stealing "bsf" options.
---
 libavformat/tee.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Steven Liu Oct. 7, 2019, 5:25 a.m. UTC | #1
> 在 2019年10月6日,上午11:37,Jun Li <junli1026@gmail.com> 写道:
> 
> Fix #7620
> In the case tee muxer with both "bsf" and "use_fifo" parameters
> wil trigger this bug. Tee muxer will first steal parameters (like "f",
> "select"...) and then "use_fifo" will try reading out remaining options
> and pass them to fifo as option "format_options".
> Current code miss the part of stealing "bsf" options.
> ---
> libavformat/tee.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index 89a4ceb280..3530582dbd 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
> static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> {
>     int i, ret;
> -    AVDictionary *options = NULL;
> +    AVDictionary *options = NULL, *bsf_options = NULL;
>     AVDictionaryEntry *entry;
>     char *filename;
>     char *format = NULL, *select = NULL, *on_fail = NULL;
> @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>     STEAL_OPTION("onfail", on_fail);
>     STEAL_OPTION("use_fifo", use_fifo);
>     STEAL_OPTION("fifo_options", fifo_options_str);
> +    entry = NULL;
> +    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {
> +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
> +        av_dict_set(&options, entry->key, NULL, 0);
> +    }
> 
>     ret = parse_slave_failure_policy_option(on_fail, tee_slave);
>     if (ret < 0) {
> @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>     }
> 
>     entry = NULL;
> -    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> +    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
>         const char *spec = entry->key + strlen("bsfs");
>         if (*spec) {
>             if (strspn(spec, slave_bsfs_spec_sep) != 1) {
> @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>             }
>         }
> 
> -        av_dict_set(&options, entry->key, NULL, 0);
> +        av_dict_set(&bsf_options, entry->key, NULL, 0);
>     }
> 
>     for (i = 0; i < avf->nb_streams; i++){
> @@ -399,6 +404,7 @@ end:
>     av_free(select);
>     av_free(on_fail);
>     av_dict_free(&options);
> +    av_dict_free(&bsf_options);
>     av_freep(&tmp_select);
>     return ret;
> }
> -- 
> 2.17.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe”.

should be ok


Thanks
Steven
Dennis Mungai Oct. 7, 2019, 9:54 p.m. UTC | #2
On Mon, 7 Oct 2019 at 08:26, Liu Steven <lq@chinaffmpeg.org> wrote:
>
>
>
> > 在 2019年10月6日,上午11:37,Jun Li <junli1026@gmail.com> 写道:
> >
> > Fix #7620
> > In the case tee muxer with both "bsf" and "use_fifo" parameters
> > wil trigger this bug. Tee muxer will first steal parameters (like "f",
> > "select"...) and then "use_fifo" will try reading out remaining options
> > and pass them to fifo as option "format_options".
> > Current code miss the part of stealing "bsf" options.
> > ---
> > libavformat/tee.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/tee.c b/libavformat/tee.c
> > index 89a4ceb280..3530582dbd 100644
> > --- a/libavformat/tee.c
> > +++ b/libavformat/tee.c
> > @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
> > static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> > {
> >     int i, ret;
> > -    AVDictionary *options = NULL;
> > +    AVDictionary *options = NULL, *bsf_options = NULL;
> >     AVDictionaryEntry *entry;
> >     char *filename;
> >     char *format = NULL, *select = NULL, *on_fail = NULL;
> > @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >     STEAL_OPTION("onfail", on_fail);
> >     STEAL_OPTION("use_fifo", use_fifo);
> >     STEAL_OPTION("fifo_options", fifo_options_str);
> > +    entry = NULL;
> > +    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {
> > +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
> > +        av_dict_set(&options, entry->key, NULL, 0);
> > +    }
> >
> >     ret = parse_slave_failure_policy_option(on_fail, tee_slave);
> >     if (ret < 0) {
> > @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >     }
> >
> >     entry = NULL;
> > -    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> > +    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> >         const char *spec = entry->key + strlen("bsfs");
> >         if (*spec) {
> >             if (strspn(spec, slave_bsfs_spec_sep) != 1) {
> > @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >             }
> >         }
> >
> > -        av_dict_set(&options, entry->key, NULL, 0);
> > +        av_dict_set(&bsf_options, entry->key, NULL, 0);
> >     }
> >
> >     for (i = 0; i < avf->nb_streams; i++){
> > @@ -399,6 +404,7 @@ end:
> >     av_free(select);
> >     av_free(on_fail);
> >     av_dict_free(&options);
> > +    av_dict_free(&bsf_options);
> >     av_freep(&tmp_select);
> >     return ret;
> > }
> > --
> > 2.17.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe”.
>
> should be ok
>
>
> Thanks
> Steven
>
>
>
Generating valid HLS + DASH streams using fragmented mp4 should be
much easier now, thanks.
Steven Liu Oct. 8, 2019, 5:51 a.m. UTC | #3
> 在 2019年10月8日,05:54,Dennis Mungai <dmngaie@gmail.com> 写道:
> 
> On Mon, 7 Oct 2019 at 08:26, Liu Steven <lq@chinaffmpeg.org> wrote:
>> 
>> 
>> 
>>> 在 2019年10月6日,上午11:37,Jun Li <junli1026@gmail.com> 写道:
>>> 
>>> Fix #7620
>>> In the case tee muxer with both "bsf" and "use_fifo" parameters
>>> wil trigger this bug. Tee muxer will first steal parameters (like "f",
>>> "select"...) and then "use_fifo" will try reading out remaining options
>>> and pass them to fifo as option "format_options".
>>> Current code miss the part of stealing "bsf" options.
>>> ---
>>> libavformat/tee.c | 12 +++++++++---
>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/libavformat/tee.c b/libavformat/tee.c
>>> index 89a4ceb280..3530582dbd 100644
>>> --- a/libavformat/tee.c
>>> +++ b/libavformat/tee.c
>>> @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
>>> static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>> {
>>>    int i, ret;
>>> -    AVDictionary *options = NULL;
>>> +    AVDictionary *options = NULL, *bsf_options = NULL;
>>>    AVDictionaryEntry *entry;
>>>    char *filename;
>>>    char *format = NULL, *select = NULL, *on_fail = NULL;
>>> @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>    STEAL_OPTION("onfail", on_fail);
>>>    STEAL_OPTION("use_fifo", use_fifo);
>>>    STEAL_OPTION("fifo_options", fifo_options_str);
>>> +    entry = NULL;
>>> +    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {
>>> +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
>>> +        av_dict_set(&options, entry->key, NULL, 0);
>>> +    }
>>> 
>>>    ret = parse_slave_failure_policy_option(on_fail, tee_slave);
>>>    if (ret < 0) {
>>> @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>    }
>>> 
>>>    entry = NULL;
>>> -    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
>>> +    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
>>>        const char *spec = entry->key + strlen("bsfs");
>>>        if (*spec) {
>>>            if (strspn(spec, slave_bsfs_spec_sep) != 1) {
>>> @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>            }
>>>        }
>>> 
>>> -        av_dict_set(&options, entry->key, NULL, 0);
>>> +        av_dict_set(&bsf_options, entry->key, NULL, 0);
>>>    }
>>> 
>>>    for (i = 0; i < avf->nb_streams; i++){
>>> @@ -399,6 +404,7 @@ end:
>>>    av_free(select);
>>>    av_free(on_fail);
>>>    av_dict_free(&options);
>>> +    av_dict_free(&bsf_options);
>>>    av_freep(&tmp_select);
>>>    return ret;
>>> }
>>> --
>>> 2.17.1
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe”.
>> 
>> should be ok
>> 
>> 
>> Thanks
>> Steven
>> 
>> 
>> 
> Generating valid HLS + DASH streams using fragmented mp4 should be
> much easier now, thanks.
Do you mean much easier after this patch?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven
Dennis Mungai Oct. 8, 2019, 5:51 p.m. UTC | #4
On Tue, 8 Oct 2019 at 08:53, Steven Liu <lq@chinaffmpeg.org> wrote:
>
>
>
> > 在 2019年10月8日,05:54,Dennis Mungai <dmngaie@gmail.com> 写道:
> >
> > On Mon, 7 Oct 2019 at 08:26, Liu Steven <lq@chinaffmpeg.org> wrote:
> >>
> >>
> >>
> >>> 在 2019年10月6日,上午11:37,Jun Li <junli1026@gmail.com> 写道:
> >>>
> >>> Fix #7620
> >>> In the case tee muxer with both "bsf" and "use_fifo" parameters
> >>> wil trigger this bug. Tee muxer will first steal parameters (like "f",
> >>> "select"...) and then "use_fifo" will try reading out remaining options
> >>> and pass them to fifo as option "format_options".
> >>> Current code miss the part of stealing "bsf" options.
> >>> ---
> >>> libavformat/tee.c | 12 +++++++++---
> >>> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/libavformat/tee.c b/libavformat/tee.c
> >>> index 89a4ceb280..3530582dbd 100644
> >>> --- a/libavformat/tee.c
> >>> +++ b/libavformat/tee.c
> >>> @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
> >>> static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >>> {
> >>>    int i, ret;
> >>> -    AVDictionary *options = NULL;
> >>> +    AVDictionary *options = NULL, *bsf_options = NULL;
> >>>    AVDictionaryEntry *entry;
> >>>    char *filename;
> >>>    char *format = NULL, *select = NULL, *on_fail = NULL;
> >>> @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >>>    STEAL_OPTION("onfail", on_fail);
> >>>    STEAL_OPTION("use_fifo", use_fifo);
> >>>    STEAL_OPTION("fifo_options", fifo_options_str);
> >>> +    entry = NULL;
> >>> +    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {
> >>> +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
> >>> +        av_dict_set(&options, entry->key, NULL, 0);
> >>> +    }
> >>>
> >>>    ret = parse_slave_failure_policy_option(on_fail, tee_slave);
> >>>    if (ret < 0) {
> >>> @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >>>    }
> >>>
> >>>    entry = NULL;
> >>> -    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> >>> +    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> >>>        const char *spec = entry->key + strlen("bsfs");
> >>>        if (*spec) {
> >>>            if (strspn(spec, slave_bsfs_spec_sep) != 1) {
> >>> @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >>>            }
> >>>        }
> >>>
> >>> -        av_dict_set(&options, entry->key, NULL, 0);
> >>> +        av_dict_set(&bsf_options, entry->key, NULL, 0);
> >>>    }
> >>>
> >>>    for (i = 0; i < avf->nb_streams; i++){
> >>> @@ -399,6 +404,7 @@ end:
> >>>    av_free(select);
> >>>    av_free(on_fail);
> >>>    av_dict_free(&options);
> >>> +    av_dict_free(&bsf_options);
> >>>    av_freep(&tmp_select);
> >>>    return ret;
> >>> }
> >>> --
> >>> 2.17.1
> >>>
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel@ffmpeg.org
> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>> To unsubscribe, visit link above, or email
> >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe”.
> >>
> >> should be ok
> >>
> >>
> >> Thanks
> >> Steven
> >>
> >>
> >>
> > Generating valid HLS + DASH streams using fragmented mp4 should be
> > much easier now, thanks.
> Do you mean much easier after this patch?
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
> Thanks
> Steven

Yes, Steven.
Previously, attempting to use fifo within the tee muxer with any BSF
applied resulted in error.
And this patch fixes that, allowing for fifo-enabled tee slaves to
resume operations with BSFs in place.
Steven Liu Oct. 12, 2019, 3:56 a.m. UTC | #5
> 在 2019年10月9日,01:51,Dennis Mungai <dmngaie@gmail.com> 写道:
> 
> On Tue, 8 Oct 2019 at 08:53, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> 
>> 
>>> 在 2019年10月8日,05:54,Dennis Mungai <dmngaie@gmail.com> 写道:
>>> 
>>> On Mon, 7 Oct 2019 at 08:26, Liu Steven <lq@chinaffmpeg.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> 在 2019年10月6日,上午11:37,Jun Li <junli1026@gmail.com> 写道:
>>>>> 
>>>>> Fix #7620
>>>>> In the case tee muxer with both "bsf" and "use_fifo" parameters
>>>>> wil trigger this bug. Tee muxer will first steal parameters (like "f",
>>>>> "select"...) and then "use_fifo" will try reading out remaining options
>>>>> and pass them to fifo as option "format_options".
>>>>> Current code miss the part of stealing "bsf" options.
>>>>> ---
>>>>> libavformat/tee.c | 12 +++++++++---
>>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>> 
>>>>> diff --git a/libavformat/tee.c b/libavformat/tee.c
>>>>> index 89a4ceb280..3530582dbd 100644
>>>>> --- a/libavformat/tee.c
>>>>> +++ b/libavformat/tee.c
>>>>> @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
>>>>> static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>>> {
>>>>>   int i, ret;
>>>>> -    AVDictionary *options = NULL;
>>>>> +    AVDictionary *options = NULL, *bsf_options = NULL;
>>>>>   AVDictionaryEntry *entry;
>>>>>   char *filename;
>>>>>   char *format = NULL, *select = NULL, *on_fail = NULL;
>>>>> @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>>>   STEAL_OPTION("onfail", on_fail);
>>>>>   STEAL_OPTION("use_fifo", use_fifo);
>>>>>   STEAL_OPTION("fifo_options", fifo_options_str);
>>>>> +    entry = NULL;
>>>>> +    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {
>>>>> +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
>>>>> +        av_dict_set(&options, entry->key, NULL, 0);
>>>>> +    }
>>>>> 
>>>>>   ret = parse_slave_failure_policy_option(on_fail, tee_slave);
>>>>>   if (ret < 0) {
>>>>> @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>>>   }
>>>>> 
>>>>>   entry = NULL;
>>>>> -    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
>>>>> +    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
>>>>>       const char *spec = entry->key + strlen("bsfs");
>>>>>       if (*spec) {
>>>>>           if (strspn(spec, slave_bsfs_spec_sep) != 1) {
>>>>> @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>>>           }
>>>>>       }
>>>>> 
>>>>> -        av_dict_set(&options, entry->key, NULL, 0);
>>>>> +        av_dict_set(&bsf_options, entry->key, NULL, 0);
>>>>>   }
>>>>> 
>>>>>   for (i = 0; i < avf->nb_streams; i++){
>>>>> @@ -399,6 +404,7 @@ end:
>>>>>   av_free(select);
>>>>>   av_free(on_fail);
>>>>>   av_dict_free(&options);
>>>>> +    av_dict_free(&bsf_options);
>>>>>   av_freep(&tmp_select);
>>>>>   return ret;
>>>>> }
>>>>> --
>>>>> 2.17.1
>>>>> 
>>>>> _______________________________________________
>>>>> ffmpeg-devel mailing list
>>>>> ffmpeg-devel@ffmpeg.org
>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>> 
>>>>> To unsubscribe, visit link above, or email
>>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe”.
>>>> 
>>>> should be ok
>>>> 
>>>> 
>>>> Thanks
>>>> Steven
>>>> 
>>>> 
>>>> 
>>> Generating valid HLS + DASH streams using fragmented mp4 should be
>>> much easier now, thanks.
>> Do you mean much easier after this patch?
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
>> Thanks
>> Steven
> 
> Yes, Steven.
> Previously, attempting to use fifo within the tee muxer with any BSF
> applied resulted in error.
> And this patch fixes that, allowing for fifo-enabled tee slaves to
> resume operations with BSFs in place.
Will push if there have no objections.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven
Dennis Mungai Oct. 12, 2019, 5:15 a.m. UTC | #6
On Sat, 12 Oct 2019, 06:57 Steven Liu, <lq@chinaffmpeg.org> wrote:

>
>
> > 在 2019年10月9日,01:51,Dennis Mungai <dmngaie@gmail.com> 写道:
> >
> > On Tue, 8 Oct 2019 at 08:53, Steven Liu <lq@chinaffmpeg.org> wrote:
> >>
> >>
> >>
> >>> 在 2019年10月8日,05:54,Dennis Mungai <dmngaie@gmail.com> 写道:
> >>>
> >>> On Mon, 7 Oct 2019 at 08:26, Liu Steven <lq@chinaffmpeg.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> 在 2019年10月6日,上午11:37,Jun Li <junli1026@gmail.com> 写道:
> >>>>>
> >>>>> Fix #7620
> >>>>> In the case tee muxer with both "bsf" and "use_fifo" parameters
> >>>>> wil trigger this bug. Tee muxer will first steal parameters (like
> "f",
> >>>>> "select"...) and then "use_fifo" will try reading out remaining
> options
> >>>>> and pass them to fifo as option "format_options".
> >>>>> Current code miss the part of stealing "bsf" options.
> >>>>> ---
> >>>>> libavformat/tee.c | 12 +++++++++---
> >>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/libavformat/tee.c b/libavformat/tee.c
> >>>>> index 89a4ceb280..3530582dbd 100644
> >>>>> --- a/libavformat/tee.c
> >>>>> +++ b/libavformat/tee.c
> >>>>> @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
> >>>>> static int open_slave(AVFormatContext *avf, char *slave, TeeSlave
> *tee_slave)
> >>>>> {
> >>>>>   int i, ret;
> >>>>> -    AVDictionary *options = NULL;
> >>>>> +    AVDictionary *options = NULL, *bsf_options = NULL;
> >>>>>   AVDictionaryEntry *entry;
> >>>>>   char *filename;
> >>>>>   char *format = NULL, *select = NULL, *on_fail = NULL;
> >>>>> @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf,
> char *slave, TeeSlave *tee_slave)
> >>>>>   STEAL_OPTION("onfail", on_fail);
> >>>>>   STEAL_OPTION("use_fifo", use_fifo);
> >>>>>   STEAL_OPTION("fifo_options", fifo_options_str);
> >>>>> +    entry = NULL;
> >>>>> +    while ((entry = av_dict_get(options, "bsfs", entry,
> AV_DICT_IGNORE_SUFFIX))) {
> >>>>> +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
> >>>>> +        av_dict_set(&options, entry->key, NULL, 0);
> >>>>> +    }
> >>>>>
> >>>>>   ret = parse_slave_failure_policy_option(on_fail, tee_slave);
> >>>>>   if (ret < 0) {
> >>>>> @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char
> *slave, TeeSlave *tee_slave)
> >>>>>   }
> >>>>>
> >>>>>   entry = NULL;
> >>>>> -    while (entry = av_dict_get(options, "bsfs", NULL,
> AV_DICT_IGNORE_SUFFIX)) {
> >>>>> +    while (entry = av_dict_get(bsf_options, "bsfs", NULL,
> AV_DICT_IGNORE_SUFFIX)) {
> >>>>>       const char *spec = entry->key + strlen("bsfs");
> >>>>>       if (*spec) {
> >>>>>           if (strspn(spec, slave_bsfs_spec_sep) != 1) {
> >>>>> @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char
> *slave, TeeSlave *tee_slave)
> >>>>>           }
> >>>>>       }
> >>>>>
> >>>>> -        av_dict_set(&options, entry->key, NULL, 0);
> >>>>> +        av_dict_set(&bsf_options, entry->key, NULL, 0);
> >>>>>   }
> >>>>>
> >>>>>   for (i = 0; i < avf->nb_streams; i++){
> >>>>> @@ -399,6 +404,7 @@ end:
> >>>>>   av_free(select);
> >>>>>   av_free(on_fail);
> >>>>>   av_dict_free(&options);
> >>>>> +    av_dict_free(&bsf_options);
> >>>>>   av_freep(&tmp_select);
> >>>>>   return ret;
> >>>>> }
> >>>>> --
> >>>>> 2.17.1
> >>>>>
> >>>>> _______________________________________________
> >>>>> ffmpeg-devel mailing list
> >>>>> ffmpeg-devel@ffmpeg.org
> >>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>>>
> >>>>> To unsubscribe, visit link above, or email
> >>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe”.
> >>>>
> >>>> should be ok
> >>>>
> >>>>
> >>>> Thanks
> >>>> Steven
> >>>>
> >>>>
> >>>>
> >>> Generating valid HLS + DASH streams using fragmented mp4 should be
> >>> much easier now, thanks.
> >> Do you mean much easier after this patch?
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel@ffmpeg.org
> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>> To unsubscribe, visit link above, or email
> >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >>
> >> Thanks
> >> Steven
> >
> > Yes, Steven.
> > Previously, attempting to use fifo within the tee muxer with any BSF
> > applied resulted in error.
> > And this patch fixes that, allowing for fifo-enabled tee slaves to
> > resume operations with BSFs in place.
> Will push if there have no objections.
>
>
> Thanks.
Nicolas George Oct. 13, 2019, 5:39 p.m. UTC | #7
Jun Li (12019-10-05):
> Fix #7620
> In the case tee muxer with both "bsf" and "use_fifo" parameters
> wil trigger this bug. Tee muxer will first steal parameters (like "f",

will

> "select"...) and then "use_fifo" will try reading out remaining options
> and pass them to fifo as option "format_options".
> Current code miss the part of stealing "bsf" options.
> ---
>  libavformat/tee.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index 89a4ceb280..3530582dbd 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
>  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>  {
>      int i, ret;
> -    AVDictionary *options = NULL;
> +    AVDictionary *options = NULL, *bsf_options = NULL;
>      AVDictionaryEntry *entry;
>      char *filename;
>      char *format = NULL, *select = NULL, *on_fail = NULL;
> @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>      STEAL_OPTION("onfail", on_fail);
>      STEAL_OPTION("use_fifo", use_fifo);
>      STEAL_OPTION("fifo_options", fifo_options_str);
> +    entry = NULL;
> +    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {

> +        av_dict_set(&bsf_options, entry->key, entry->value, 0);

You could use entry->key + 4 to trim the "bsfs" prefix.

> +        av_dict_set(&options, entry->key, NULL, 0);
> +    }
>  
>      ret = parse_slave_failure_policy_option(on_fail, tee_slave);
>      if (ret < 0) {
> @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>      }
>  
>      entry = NULL;
> -    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> +    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
>          const char *spec = entry->key + strlen("bsfs");
>          if (*spec) {
>              if (strspn(spec, slave_bsfs_spec_sep) != 1) {
> @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>              }
>          }
>  
> -        av_dict_set(&options, entry->key, NULL, 0);
> +        av_dict_set(&bsf_options, entry->key, NULL, 0);
>      }
>  
>      for (i = 0; i < avf->nb_streams; i++){
> @@ -399,6 +404,7 @@ end:
>      av_free(select);
>      av_free(on_fail);
>      av_dict_free(&options);
> +    av_dict_free(&bsf_options);
>      av_freep(&tmp_select);
>      return ret;
>  }

Apart from that, LGTM.

Regards,
Jun Li Oct. 13, 2019, 9:16 p.m. UTC | #8
On Sun, Oct 13, 2019 at 10:39 AM Nicolas George <george@nsup.org> wrote:

> Jun Li (12019-10-05):
> > Fix #7620
> > In the case tee muxer with both "bsf" and "use_fifo" parameters
> > wil trigger this bug. Tee muxer will first steal parameters (like "f",
>
> will
>
> > "select"...) and then "use_fifo" will try reading out remaining options
> > and pass them to fifo as option "format_options".
> > Current code miss the part of stealing "bsf" options.
> > ---
> >  libavformat/tee.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/tee.c b/libavformat/tee.c
> > index 89a4ceb280..3530582dbd 100644
> > --- a/libavformat/tee.c
> > +++ b/libavformat/tee.c
> > @@ -159,7 +159,7 @@ static void close_slaves(AVFormatContext *avf)
> >  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave
> *tee_slave)
> >  {
> >      int i, ret;
> > -    AVDictionary *options = NULL;
> > +    AVDictionary *options = NULL, *bsf_options = NULL;
> >      AVDictionaryEntry *entry;
> >      char *filename;
> >      char *format = NULL, *select = NULL, *on_fail = NULL;
> > @@ -186,6 +186,11 @@ static int open_slave(AVFormatContext *avf, char
> *slave, TeeSlave *tee_slave)
> >      STEAL_OPTION("onfail", on_fail);
> >      STEAL_OPTION("use_fifo", use_fifo);
> >      STEAL_OPTION("fifo_options", fifo_options_str);
> > +    entry = NULL;
> > +    while ((entry = av_dict_get(options, "bsfs", entry,
> AV_DICT_IGNORE_SUFFIX))) {
>
> > +        av_dict_set(&bsf_options, entry->key, entry->value, 0);
>
> You could use entry->key + 4 to trim the "bsfs" prefix.


Thanks for review, version 2 is sent out for addressing it.
https://patchwork.ffmpeg.org/patch/15731/


>
>
> +        av_dict_set(&options, entry->key, NULL, 0);
> > +    }
> >
> >      ret = parse_slave_failure_policy_option(on_fail, tee_slave);
> >      if (ret < 0) {
> > @@ -311,7 +316,7 @@ static int open_slave(AVFormatContext *avf, char
> *slave, TeeSlave *tee_slave)
> >      }
> >
> >      entry = NULL;
> > -    while (entry = av_dict_get(options, "bsfs", NULL,
> AV_DICT_IGNORE_SUFFIX)) {
> > +    while (entry = av_dict_get(bsf_options, "bsfs", NULL,
> AV_DICT_IGNORE_SUFFIX)) {
> >          const char *spec = entry->key + strlen("bsfs");
> >          if (*spec) {
> >              if (strspn(spec, slave_bsfs_spec_sep) != 1) {
> > @@ -352,7 +357,7 @@ static int open_slave(AVFormatContext *avf, char
> *slave, TeeSlave *tee_slave)
> >              }
> >          }
> >
> > -        av_dict_set(&options, entry->key, NULL, 0);
> > +        av_dict_set(&bsf_options, entry->key, NULL, 0);
> >      }
> >
> >      for (i = 0; i < avf->nb_streams; i++){
> > @@ -399,6 +404,7 @@ end:
> >      av_free(select);
> >      av_free(on_fail);
> >      av_dict_free(&options);
> > +    av_dict_free(&bsf_options);
> >      av_freep(&tmp_select);
> >      return ret;
> >  }
>
> Apart from that, LGTM.
>
> Regards,
>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox

Patch

diff --git a/libavformat/tee.c b/libavformat/tee.c
index 89a4ceb280..3530582dbd 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -159,7 +159,7 @@  static void close_slaves(AVFormatContext *avf)
 static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
 {
     int i, ret;
-    AVDictionary *options = NULL;
+    AVDictionary *options = NULL, *bsf_options = NULL;
     AVDictionaryEntry *entry;
     char *filename;
     char *format = NULL, *select = NULL, *on_fail = NULL;
@@ -186,6 +186,11 @@  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
     STEAL_OPTION("onfail", on_fail);
     STEAL_OPTION("use_fifo", use_fifo);
     STEAL_OPTION("fifo_options", fifo_options_str);
+    entry = NULL;
+    while ((entry = av_dict_get(options, "bsfs", entry, AV_DICT_IGNORE_SUFFIX))) {
+        av_dict_set(&bsf_options, entry->key, entry->value, 0);
+        av_dict_set(&options, entry->key, NULL, 0);
+    }
 
     ret = parse_slave_failure_policy_option(on_fail, tee_slave);
     if (ret < 0) {
@@ -311,7 +316,7 @@  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
     }
 
     entry = NULL;
-    while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
+    while (entry = av_dict_get(bsf_options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
         const char *spec = entry->key + strlen("bsfs");
         if (*spec) {
             if (strspn(spec, slave_bsfs_spec_sep) != 1) {
@@ -352,7 +357,7 @@  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
             }
         }
 
-        av_dict_set(&options, entry->key, NULL, 0);
+        av_dict_set(&bsf_options, entry->key, NULL, 0);
     }
 
     for (i = 0; i < avf->nb_streams; i++){
@@ -399,6 +404,7 @@  end:
     av_free(select);
     av_free(on_fail);
     av_dict_free(&options);
+    av_dict_free(&bsf_options);
     av_freep(&tmp_select);
     return ret;
 }