[FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present

[Leo Izen]

On 7/9/24 1:21 PM, James Almer wrote:
> It's meant to override any codec specific (but container level) information,
> but its position is not guaranteed, so apply the values after the entire trak
> structure has been parsed.
> Also, replace the ugly roundabout int -> double -> int method to set SAR from
> existing dimensions while at it.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>   libavformat/isom.h |  2 ++
>   libavformat/mov.c  | 16 ++++++++++++----
>   2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index a0498f45e5..5b6125a908 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -215,6 +215,8 @@ typedef struct MOVStreamContext {
>       int timecode_track;
>       int width;            ///< tkhd width
>       int height;           ///< tkhd height
> +    int h_spacing;        ///< pasp hSpacing
> +    int v_spacing;        ///< pasp vSpacing
>       int dts_shift;        ///< dts shift when ctts is negative
>       uint32_t palette[256];
>       int has_palette;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index ced4b2e6b3..ce95842ce5 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1287,14 +1287,18 @@ static int mov_read_pasp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>       const int num = avio_rb32(pb);
>       const int den = avio_rb32(pb);
>       AVStream *st;
> +    MOVStreamContext *sc;
>   
>       if (c->fc->nb_streams < 1)
>           return 0;
>       st = c->fc->streams[c->fc->nb_streams-1];
> +    sc = st->priv_data;
> +
> +    av_log(c->fc, AV_LOG_TRACE, "pasp: hSpacing %d, vSpacing %d\n", num, den);
>   
>       if (den != 0) {
> -        av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den,
> -                  num, den, 32767);
> +        sc->h_spacing = num;
> +        sc->v_spacing = den;
>       }

If den == 0, nothing is assigned at all, but if den != 0 and if num == 
0, we assign 0 to h_spacing, which will end up doing nothing because we 
check h_spacing && v_spacing later on in the other line you add.

Is this intentional? Is this desired behavior? etc.

- Leo Izen (Traneptora)