[FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[pal]

From: Pierre-Anthony Lemieux <pal@palemieux.com>

As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
currently preserve stream information such as language in the case of audio
streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.

---
 libavformat/imfdec.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
index 71dfb26958..7aa66a06bf 100644
--- a/libavformat/imfdec.c
+++ b/libavformat/imfdec.c
@@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
             return AVERROR(ENOMEM);
         }
         asset_stream->id = i;
+        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
+        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
+        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
         ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
         if (ret < 0) {
             av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
             return ret;
         }
+        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
+        ff_stream_side_data_copy(asset_stream, first_resource_stream);
         avpriv_set_pts_info(asset_stream,
                             first_resource_stream->pts_wrap_bits,
                             first_resource_stream->time_base.num,
-- 
2.25.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".

Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[Andreas Rheinhardt]

pal@sandflow.com:
> From: Pierre-Anthony Lemieux <pal@palemieux.com>
> 
> As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
> currently preserve stream information such as language in the case of audio
> streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.
> 
> ---
>  libavformat/imfdec.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> index 71dfb26958..7aa66a06bf 100644
> --- a/libavformat/imfdec.c
> +++ b/libavformat/imfdec.c
> @@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
>              return AVERROR(ENOMEM);
>          }
>          asset_stream->id = i;
> +        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
> +        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
> +        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
>          ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
>          if (ret < 0) {
>              av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
>              return ret;
>          }
> +        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
> +        ff_stream_side_data_copy(asset_stream, first_resource_stream);
>          avpriv_set_pts_info(asset_stream,
>                              first_resource_stream->pts_wrap_bits,
>                              first_resource_stream->time_base.num,

Seems to me like one should use ff_stream_encode_params_copy here. Of
course, it would have to be renamed and moved if used in a demuxer.

- Andreas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[Pierre-Anthony Lemieux]

On Sun, Jul 3, 2022 at 11:28 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> pal@sandflow.com:
> > From: Pierre-Anthony Lemieux <pal@palemieux.com>
> >
> > As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
> > currently preserve stream information such as language in the case of audio
> > streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.
> >
> > ---
> >  libavformat/imfdec.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> > index 71dfb26958..7aa66a06bf 100644
> > --- a/libavformat/imfdec.c
> > +++ b/libavformat/imfdec.c
> > @@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
> >              return AVERROR(ENOMEM);
> >          }
> >          asset_stream->id = i;
> > +        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
> > +        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
> > +        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
> >          ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
> >          if (ret < 0) {
> >              av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
> >              return ret;
> >          }
> > +        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
> > +        ff_stream_side_data_copy(asset_stream, first_resource_stream);
> >          avpriv_set_pts_info(asset_stream,
> >                              first_resource_stream->pts_wrap_bits,
> >                              first_resource_stream->time_base.num,
>
> Seems to me like one should use ff_stream_encode_params_copy here. Of
> course, it would have to be renamed and moved if used in a demuxer.

Would copy_stream_props() in concatdec.c need to be refactored as well?

Note that, in the case of avformat/imfdec.c, AVStream::id is not
copied across, so ff_stream_encode_params_copy() would need to be
followed by asset_stream->id = i;

>
> - Andreas
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[Andreas Rheinhardt]

Pierre-Anthony Lemieux:
> On Sun, Jul 3, 2022 at 11:28 AM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> pal@sandflow.com:
>>> From: Pierre-Anthony Lemieux <pal@palemieux.com>
>>>
>>> As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
>>> currently preserve stream information such as language in the case of audio
>>> streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.
>>>
>>> ---
>>>  libavformat/imfdec.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
>>> index 71dfb26958..7aa66a06bf 100644
>>> --- a/libavformat/imfdec.c
>>> +++ b/libavformat/imfdec.c
>>> @@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
>>>              return AVERROR(ENOMEM);
>>>          }
>>>          asset_stream->id = i;
>>> +        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
>>> +        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
>>> +        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
>>>          ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
>>>          if (ret < 0) {
>>>              av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
>>>              return ret;
>>>          }
>>> +        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
>>> +        ff_stream_side_data_copy(asset_stream, first_resource_stream);
>>>          avpriv_set_pts_info(asset_stream,
>>>                              first_resource_stream->pts_wrap_bits,
>>>                              first_resource_stream->time_base.num,
>>
>> Seems to me like one should use ff_stream_encode_params_copy here. Of
>> course, it would have to be renamed and moved if used in a demuxer.
> 
> Would copy_stream_props() in concatdec.c need to be refactored as well?
> 

I often wondered about this. The problem with copy_stream_props is that
it is not only called during read_header, but lateron as well, but e.g.
the documentation of AVStream.side_data says that it is populated when
the stream is created and not later.
This issue does of course not exist in your case.

> Note that, in the case of avformat/imfdec.c, AVStream::id is not
> copied across, so ff_stream_encode_params_copy() would need to be
> followed by asset_stream->id = i;
> 

Yeah, I know.

- Andreas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[Pierre-Anthony Lemieux]

On Sun, Jul 3, 2022 at 12:15 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Pierre-Anthony Lemieux:
> > On Sun, Jul 3, 2022 at 11:28 AM Andreas Rheinhardt
> > <andreas.rheinhardt@outlook.com> wrote:
> >>
> >> pal@sandflow.com:
> >>> From: Pierre-Anthony Lemieux <pal@palemieux.com>
> >>>
> >>> As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
> >>> currently preserve stream information such as language in the case of audio
> >>> streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.
> >>>
> >>> ---
> >>>  libavformat/imfdec.c | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> >>> index 71dfb26958..7aa66a06bf 100644
> >>> --- a/libavformat/imfdec.c
> >>> +++ b/libavformat/imfdec.c
> >>> @@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
> >>>              return AVERROR(ENOMEM);
> >>>          }
> >>>          asset_stream->id = i;
> >>> +        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
> >>> +        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
> >>> +        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
> >>>          ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
> >>>          if (ret < 0) {
> >>>              av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
> >>>              return ret;
> >>>          }
> >>> +        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
> >>> +        ff_stream_side_data_copy(asset_stream, first_resource_stream);
> >>>          avpriv_set_pts_info(asset_stream,
> >>>                              first_resource_stream->pts_wrap_bits,
> >>>                              first_resource_stream->time_base.num,
> >>
> >> Seems to me like one should use ff_stream_encode_params_copy here. Of
> >> course, it would have to be renamed and moved if used in a demuxer.
> >
> > Would copy_stream_props() in concatdec.c need to be refactored as well?
> >
>
> I often wondered about this. The problem with copy_stream_props is that
> it is not only called during read_header, but lateron as well, but e.g.
> the documentation of AVStream.side_data says that it is populated when
> the stream is created and not later.
> This issue does of course not exist in your case.

ff_stream_encode_params_copy() does not seem to set pts_wrap_bits,
i.e. it does not call avpriv_set_pts_info().

>
> > Note that, in the case of avformat/imfdec.c, AVStream::id is not
> > copied across, so ff_stream_encode_params_copy() would need to be
> > followed by asset_stream->id = i;
> >
>
> Yeah, I know.
>
> - Andreas
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[Andreas Rheinhardt]

Pierre-Anthony Lemieux:
> On Sun, Jul 3, 2022 at 12:15 PM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> Pierre-Anthony Lemieux:
>>> On Sun, Jul 3, 2022 at 11:28 AM Andreas Rheinhardt
>>> <andreas.rheinhardt@outlook.com> wrote:
>>>>
>>>> pal@sandflow.com:
>>>>> From: Pierre-Anthony Lemieux <pal@palemieux.com>
>>>>>
>>>>> As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
>>>>> currently preserve stream information such as language in the case of audio
>>>>> streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.
>>>>>
>>>>> ---
>>>>>  libavformat/imfdec.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
>>>>> index 71dfb26958..7aa66a06bf 100644
>>>>> --- a/libavformat/imfdec.c
>>>>> +++ b/libavformat/imfdec.c
>>>>> @@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
>>>>>              return AVERROR(ENOMEM);
>>>>>          }
>>>>>          asset_stream->id = i;
>>>>> +        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
>>>>> +        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
>>>>> +        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
>>>>>          ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
>>>>>          if (ret < 0) {
>>>>>              av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
>>>>>              return ret;
>>>>>          }
>>>>> +        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
>>>>> +        ff_stream_side_data_copy(asset_stream, first_resource_stream);
>>>>>          avpriv_set_pts_info(asset_stream,
>>>>>                              first_resource_stream->pts_wrap_bits,
>>>>>                              first_resource_stream->time_base.num,
>>>>
>>>> Seems to me like one should use ff_stream_encode_params_copy here. Of
>>>> course, it would have to be renamed and moved if used in a demuxer.
>>>
>>> Would copy_stream_props() in concatdec.c need to be refactored as well?
>>>
>>
>> I often wondered about this. The problem with copy_stream_props is that
>> it is not only called during read_header, but lateron as well, but e.g.
>> the documentation of AVStream.side_data says that it is populated when
>> the stream is created and not later.
>> This issue does of course not exist in your case.
> 
> ff_stream_encode_params_copy() does not seem to set pts_wrap_bits,
> i.e. it does not call avpriv_set_pts_info().
> 

The reason for this is that pts_wrap_bits is irrelevant for muxing.

>>
>>> Note that, in the case of avformat/imfdec.c, AVStream::id is not
>>> copied across, so ff_stream_encode_params_copy() would need to be
>>> followed by asset_stream->id = i;
>>>
>>
>> Yeah, I know.
>>
>> - Andreas
>> _______________________________________________
>> 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".
> _______________________________________________
> 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".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information

[Pierre-Anthony Lemieux]

On Sun, Jul 3, 2022 at 2:13 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Pierre-Anthony Lemieux:
> > On Sun, Jul 3, 2022 at 12:15 PM Andreas Rheinhardt
> > <andreas.rheinhardt@outlook.com> wrote:
> >>
> >> Pierre-Anthony Lemieux:
> >>> On Sun, Jul 3, 2022 at 11:28 AM Andreas Rheinhardt
> >>> <andreas.rheinhardt@outlook.com> wrote:
> >>>>
> >>>> pal@sandflow.com:
> >>>>> From: Pierre-Anthony Lemieux <pal@palemieux.com>
> >>>>>
> >>>>> As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer does not
> >>>>> currently preserve stream information such as language in the case of audio
> >>>>> streams. This patch is modeled on copy_stream_props() at avformat/concatdec.c.
> >>>>>
> >>>>> ---
> >>>>>  libavformat/imfdec.c | 5 +++++
> >>>>>  1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> >>>>> index 71dfb26958..7aa66a06bf 100644
> >>>>> --- a/libavformat/imfdec.c
> >>>>> +++ b/libavformat/imfdec.c
> >>>>> @@ -580,11 +580,16 @@ static int set_context_streams_from_tracks(AVFormatContext *s)
> >>>>>              return AVERROR(ENOMEM);
> >>>>>          }
> >>>>>          asset_stream->id = i;
> >>>>> +        asset_stream->r_frame_rate = first_resource_stream->r_frame_rate;
> >>>>> +        asset_stream->avg_frame_rate = first_resource_stream->avg_frame_rate;
> >>>>> +        asset_stream->sample_aspect_ratio = first_resource_stream->sample_aspect_ratio;
> >>>>>          ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar);
> >>>>>          if (ret < 0) {
> >>>>>              av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n");
> >>>>>              return ret;
> >>>>>          }
> >>>>> +        av_dict_copy(&asset_stream->metadata, first_resource_stream->metadata, 0);
> >>>>> +        ff_stream_side_data_copy(asset_stream, first_resource_stream);
> >>>>>          avpriv_set_pts_info(asset_stream,
> >>>>>                              first_resource_stream->pts_wrap_bits,
> >>>>>                              first_resource_stream->time_base.num,
> >>>>
> >>>> Seems to me like one should use ff_stream_encode_params_copy here. Of
> >>>> course, it would have to be renamed and moved if used in a demuxer.
> >>>
> >>> Would copy_stream_props() in concatdec.c need to be refactored as well?
> >>>
> >>
> >> I often wondered about this. The problem with copy_stream_props is that
> >> it is not only called during read_header, but lateron as well, but e.g.
> >> the documentation of AVStream.side_data says that it is populated when
> >> the stream is created and not later.
> >> This issue does of course not exist in your case.
> >
> > ff_stream_encode_params_copy() does not seem to set pts_wrap_bits,
> > i.e. it does not call avpriv_set_pts_info().
> >
>
> The reason for this is that pts_wrap_bits is irrelevant for muxing.

Do you have in mind:

(a) a single av_stream_params_copy(), which copies all parameters
(b) two separate functions av_stream_demux_params_copy() and
av_stream_mux_params_copy(), which copies only relevant parameters?
(c) something else?

In the case of (b), should both be defined now, or only
av_stream_demux_params_copy() be implemented now since avformat/imfdec
would only use av_stream_demux_params_copy()?

I have not seen anyone else chime in.

>
> >>
> >>> Note that, in the case of avformat/imfdec.c, AVStream::id is not
> >>> copied across, so ff_stream_encode_params_copy() would need to be
> >>> followed by asset_stream->id = i;
> >>>
> >>
> >> Yeah, I know.
> >>
> >> - Andreas
> >> _______________________________________________
> >> 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".
> > _______________________________________________
> > 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".
>
> _______________________________________________
> 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".
_______________________________________________
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".