[FFmpeg-devel] avformat/mlpdec: fix time_base for packet timestamps

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavformat/mlpdec.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

Paul B Mahol:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavformat/mlpdec.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mlpdec.c b/libavformat/mlpdec.c
> index 8f0aabb510..f13d0fac8c 100644
> --- a/libavformat/mlpdec.c
> +++ b/libavformat/mlpdec.c
> @@ -22,8 +22,11 @@
>   */
>  
>  #include "avformat.h"
> +#include "avio_internal.h"
> +#include "internal.h"
>  #include "rawdec.h"
>  #include "libavutil/intreadwrite.h"
> +#include "libavcodec/mlp_parse.h"
>  
>  static int av_always_inline mlp_thd_probe(const AVProbeData *p, uint32_t sync)
>  {
> @@ -50,6 +53,36 @@ static int av_always_inline mlp_thd_probe(const AVProbeData *p, uint32_t sync)
>      return 0;
>  }
>  
> +static int mlp_read_header(AVFormatContext *s)
> +{
> +    int ret = ff_raw_audio_read_header(s);
> +
> +    if (ret < 0)
> +        return ret;
> +
> +    ret = ffio_ensure_seekback(s->pb, 10);
> +    if (ret == 0) {
> +        int sample_rate, type;
> +
> +        avio_skip(s->pb, 7);
> +        type = avio_r8(s->pb);
> +
> +        switch (type) {
> +        case 0xba:
> +            sample_rate = mlp_samplerate(avio_r8(s->pb) >> 4);
> +            break;
> +        case 0xbb:
> +            sample_rate = mlp_samplerate((avio_rb16(s->pb) >> 4) & 0xF);
> +            break;
> +        }
> +
> +        avpriv_set_pts_info(s->streams[0], 64, 1, sample_rate);
> +        avio_seek(s->pb, -9 - (type == 0xbb), SEEK_CUR);

This presumes that one of the two cases of the switch will be taken.
I think it would be easier if you just read the first 10 bytes into a
stack buffer and parsed from it instead. Then you can always seek back
by 10 bytes.

(I always thought that thd can have very high samplerates (up to
192kHz). Am I wrong?)

> +    }
> +
> +    return 0;
> +}
> +
>  #if CONFIG_MLP_DEMUXER
>  static int mlp_probe(const AVProbeData *p)
>  {
> @@ -60,7 +93,7 @@ const AVInputFormat ff_mlp_demuxer = {
>      .name           = "mlp",
>      .long_name      = NULL_IF_CONFIG_SMALL("raw MLP"),
>      .read_probe     = mlp_probe,
> -    .read_header    = ff_raw_audio_read_header,
> +    .read_header    = mlp_read_header,
>      .read_packet    = ff_raw_read_partial_packet,
>      .flags          = AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS,
>      .extensions     = "mlp",
> @@ -80,7 +113,7 @@ const AVInputFormat ff_truehd_demuxer = {
>      .name           = "truehd",
>      .long_name      = NULL_IF_CONFIG_SMALL("raw TrueHD"),
>      .read_probe     = thd_probe,
> -    .read_header    = ff_raw_audio_read_header,
> +    .read_header    = mlp_read_header,
>      .read_packet    = ff_raw_read_partial_packet,
>      .flags          = AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS,
>      .extensions     = "thd",
> @@ -89,4 +122,3 @@ const AVInputFormat ff_truehd_demuxer = {
>      .priv_class     = &ff_raw_demuxer_class,
>  };
>  #endif
> -
>

On Sun, Sep 5, 2021 at 6:12 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Paul B Mahol:
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > ---
> >  libavformat/mlpdec.c | 38 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/mlpdec.c b/libavformat/mlpdec.c
> > index 8f0aabb510..f13d0fac8c 100644
> > --- a/libavformat/mlpdec.c
> > +++ b/libavformat/mlpdec.c
> > @@ -22,8 +22,11 @@
> >   */
> >
> >  #include "avformat.h"
> > +#include "avio_internal.h"
> > +#include "internal.h"
> >  #include "rawdec.h"
> >  #include "libavutil/intreadwrite.h"
> > +#include "libavcodec/mlp_parse.h"
> >
> >  static int av_always_inline mlp_thd_probe(const AVProbeData *p,
> uint32_t sync)
> >  {
> > @@ -50,6 +53,36 @@ static int av_always_inline mlp_thd_probe(const
> AVProbeData *p, uint32_t sync)
> >      return 0;
> >  }
> >
> > +static int mlp_read_header(AVFormatContext *s)
> > +{
> > +    int ret = ff_raw_audio_read_header(s);
> > +
> > +    if (ret < 0)
> > +        return ret;
> > +
> > +    ret = ffio_ensure_seekback(s->pb, 10);
> > +    if (ret == 0) {
> > +        int sample_rate, type;
> > +
> > +        avio_skip(s->pb, 7);
> > +        type = avio_r8(s->pb);
> > +
> > +        switch (type) {
> > +        case 0xba:
> > +            sample_rate = mlp_samplerate(avio_r8(s->pb) >> 4);
> > +            break;
> > +        case 0xbb:
> > +            sample_rate = mlp_samplerate((avio_rb16(s->pb) >> 4) & 0xF);
> > +            break;
> > +        }
> > +
> > +        avpriv_set_pts_info(s->streams[0], 64, 1, sample_rate);
> > +        avio_seek(s->pb, -9 - (type == 0xbb), SEEK_CUR);
>
> This presumes that one of the two cases of the switch will be taken.
> I think it would be easier if you just read the first 10 bytes into a
> stack buffer and parsed from it instead. Then you can always seek back
> by 10 bytes.
>
> (I always thought that thd can have very high samplerates (up to
> 192kHz). Am I wrong?)
>

Yes, but lowest is 44100 Hz.

>
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  #if CONFIG_MLP_DEMUXER
> >  static int mlp_probe(const AVProbeData *p)
> >  {
> > @@ -60,7 +93,7 @@ const AVInputFormat ff_mlp_demuxer = {
> >      .name           = "mlp",
> >      .long_name      = NULL_IF_CONFIG_SMALL("raw MLP"),
> >      .read_probe     = mlp_probe,
> > -    .read_header    = ff_raw_audio_read_header,
> > +    .read_header    = mlp_read_header,
> >      .read_packet    = ff_raw_read_partial_packet,
> >      .flags          = AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS,
> >      .extensions     = "mlp",
> > @@ -80,7 +113,7 @@ const AVInputFormat ff_truehd_demuxer = {
> >      .name           = "truehd",
> >      .long_name      = NULL_IF_CONFIG_SMALL("raw TrueHD"),
> >      .read_probe     = thd_probe,
> > -    .read_header    = ff_raw_audio_read_header,
> > +    .read_header    = mlp_read_header,
> >      .read_packet    = ff_raw_read_partial_packet,
> >      .flags          = AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS,
> >      .extensions     = "thd",
> > @@ -89,4 +122,3 @@ const AVInputFormat ff_truehd_demuxer = {
> >      .priv_class     = &ff_raw_demuxer_class,
> >  };
> >  #endif
> > -
> >
>
> _______________________________________________
> 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".
>

Paul B Mahol:
> On Sun, Sep 5, 2021 at 6:12 PM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
> 
>> Paul B Mahol:
>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>> ---
>>>  libavformat/mlpdec.c | 38 +++++++++++++++++++++++++++++++++++---
>>>  1 file changed, 35 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavformat/mlpdec.c b/libavformat/mlpdec.c
>>> index 8f0aabb510..f13d0fac8c 100644
>>> --- a/libavformat/mlpdec.c
>>> +++ b/libavformat/mlpdec.c
>>> @@ -22,8 +22,11 @@
>>>   */
>>>
>>>  #include "avformat.h"
>>> +#include "avio_internal.h"
>>> +#include "internal.h"
>>>  #include "rawdec.h"
>>>  #include "libavutil/intreadwrite.h"
>>> +#include "libavcodec/mlp_parse.h"
>>>
>>>  static int av_always_inline mlp_thd_probe(const AVProbeData *p,
>> uint32_t sync)
>>>  {
>>> @@ -50,6 +53,36 @@ static int av_always_inline mlp_thd_probe(const
>> AVProbeData *p, uint32_t sync)
>>>      return 0;
>>>  }
>>>
>>> +static int mlp_read_header(AVFormatContext *s)
>>> +{
>>> +    int ret = ff_raw_audio_read_header(s);
>>> +
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = ffio_ensure_seekback(s->pb, 10);
>>> +    if (ret == 0) {
>>> +        int sample_rate, type;
>>> +
>>> +        avio_skip(s->pb, 7);
>>> +        type = avio_r8(s->pb);
>>> +
>>> +        switch (type) {
>>> +        case 0xba:
>>> +            sample_rate = mlp_samplerate(avio_r8(s->pb) >> 4);
>>> +            break;
>>> +        case 0xbb:
>>> +            sample_rate = mlp_samplerate((avio_rb16(s->pb) >> 4) & 0xF);
>>> +            break;
>>> +        }
>>> +
>>> +        avpriv_set_pts_info(s->streams[0], 64, 1, sample_rate);
>>> +        avio_seek(s->pb, -9 - (type == 0xbb), SEEK_CUR);
>>
>> This presumes that one of the two cases of the switch will be taken.
>> I think it would be easier if you just read the first 10 bytes into a
>> stack buffer and parsed from it instead. Then you can always seek back
>> by 10 bytes.
>>
>> (I always thought that thd can have very high samplerates (up to
>> 192kHz). Am I wrong?)
>>
> 
> Yes, but lowest is 44100 Hz.
> 
Seems like I completely misunderstood mlp_samplerate(). I forgot the
shifting.

- Andreas