* [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present
@ 2024-07-09 17:21 James Almer
2024-07-10 16:02 ` Leo Izen
2024-07-11 12:43 ` James Almer
0 siblings, 2 replies; 4+ messages in thread
From: James Almer @ 2024-07-09 17:21 UTC (permalink / raw)
To: ffmpeg-devel
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@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;
}
return 0;
}
@@ -5031,11 +5035,15 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
}
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+ if (sc->h_spacing && sc->v_spacing)
+ av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den,
+ sc->h_spacing, sc->v_spacing, INT_MAX);
if (!st->sample_aspect_ratio.num && st->codecpar->width && st->codecpar->height &&
sc->height && sc->width &&
(st->codecpar->width != sc->width || st->codecpar->height != sc->height)) {
- st->sample_aspect_ratio = av_d2q(((double)st->codecpar->height * sc->width) /
- ((double)st->codecpar->width * sc->height), INT_MAX);
+ av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den,
+ (int64_t)st->codecpar->height * sc->width,
+ (int64_t)st->codecpar->width * sc->height, INT_MAX);
}
#if FF_API_R_FRAME_RATE
--
2.45.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present
2024-07-09 17:21 [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present James Almer
@ 2024-07-10 16:02 ` Leo Izen
2024-07-10 16:09 ` James Almer
2024-07-11 12:43 ` James Almer
1 sibling, 1 reply; 4+ messages in thread
From: Leo Izen @ 2024-07-10 16:02 UTC (permalink / raw)
To: ffmpeg-devel
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@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)
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present
2024-07-10 16:02 ` Leo Izen
@ 2024-07-10 16:09 ` James Almer
0 siblings, 0 replies; 4+ messages in thread
From: James Almer @ 2024-07-10 16:09 UTC (permalink / raw)
To: ffmpeg-devel
On 7/10/2024 1:02 PM, Leo Izen wrote:
> 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@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.
A 0/x SAR is just 0, or unset, regardless of the value for den, and we
check for sar.num immediately after the "sc->h_spacing && sc->v_spacing"
one and set it to something if it's 0, so it will make no difference.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present
2024-07-09 17:21 [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present James Almer
2024-07-10 16:02 ` Leo Izen
@ 2024-07-11 12:43 ` James Almer
1 sibling, 0 replies; 4+ messages in thread
From: James Almer @ 2024-07-11 12:43 UTC (permalink / raw)
To: ffmpeg-devel
On 7/9/2024 2: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@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;
> }
> return 0;
> }
> @@ -5031,11 +5035,15 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> }
>
> if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
> + if (sc->h_spacing && sc->v_spacing)
> + av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den,
> + sc->h_spacing, sc->v_spacing, INT_MAX);
> if (!st->sample_aspect_ratio.num && st->codecpar->width && st->codecpar->height &&
> sc->height && sc->width &&
> (st->codecpar->width != sc->width || st->codecpar->height != sc->height)) {
> - st->sample_aspect_ratio = av_d2q(((double)st->codecpar->height * sc->width) /
> - ((double)st->codecpar->width * sc->height), INT_MAX);
> + av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den,
> + (int64_t)st->codecpar->height * sc->width,
> + (int64_t)st->codecpar->width * sc->height, INT_MAX);
> }
>
> #if FF_API_R_FRAME_RATE
Will apply.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-11 12:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-09 17:21 [FFmpeg-devel] [PATCH] avformat/mov: ensure pasp box derived SAR is used if present James Almer
2024-07-10 16:02 ` Leo Izen
2024-07-10 16:09 ` James Almer
2024-07-11 12:43 ` James Almer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git