On Tue, Jan 02, 2024 at 03:14:19AM +0000, aybe aybe wrote:
> This fourth and last patch is an attempt at removing the hard-coded value of 15 FPS.
> 
> In patch 1/4, although it would render video, the audio and video were not synchronized at all, now there are.
> 
> In this approach I kept it simple, grab min/max possible rates, pick min, clamp to 15/30 just in case.
> 
> It appears to work quite well, the right frame rate is picked up and both streams are in sync.
> 
> (tested against Wipeout introduction for both PAL and NTSC versions).
> 
> 
> Besides, there have been significant findings over the years regarding that format, specifically:
> https://problemkaputt.de/psxspx-macroblock-decoder-mdec.htm
> https://github.com/m35/jpsxdec/blob/readme/jpsxdec/PlayStation1_STR_format.txt
> 
> Maybe someone versed in this topic (I'm not) could further improve support of this format according these docs...
> 
> 
> 
> Signed-off-by: aybe <aybe@users.noreply.github.com>
> ---
>   libavformat/psxstr.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/psxstr.c b/libavformat/psxstr.c
> index 306a690f52..98897acde3 100644
> --- a/libavformat/psxstr.c
> +++ b/libavformat/psxstr.c
> @@ -52,6 +52,9 @@
> 
>   #define STR_MAGIC (0x80010160)
> 
> +#define MDEC_STR_FPS_MIN 15
> +#define MDEC_STR_FPS_MAX 30
> +
>   typedef struct StrChannel {
>       /* video parameters */
>       int video_stream_index;
> @@ -65,6 +68,10 @@ typedef struct StrDemuxContext {
> 
>       /* a STR file can contain up to 32 channels of data */
>       StrChannel channels[32];
> +    /* trivial FPS detection based on sectors per frame */
> +    int fps_min; /* slowest FPS found */
> +    int fps_max; /* fastest FPS found */
> +    int fps_val; /* nominal FPS value */
>   } StrDemuxContext;
> 
>   static const uint8_t sync_header[12] = {0x00,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x00};
> @@ -150,6 +157,10 @@ static int str_read_header(AVFormatContext *s)
>           str->channels[i].audio_stream_index= -1;
>       }
> 
> +    str->fps_min = INT_MAX;
> +    str->fps_max = INT_MIN;
> +    str->fps_val = 0;
> +
>       s->ctx_flags |= AVFMTCTX_NOHEADER;
> 
>       return 0;
> @@ -161,7 +172,7 @@ static int str_read_packet(AVFormatContext *s,
>       AVIOContext *pb = s->pb;
>       StrDemuxContext *str = s->priv_data;
>       unsigned char sector[RAW_CD_SECTOR_SIZE];
> -    int channel, ret;
> +    int channel, ret, sub_mode, idx_sect, num_sect;
>       AVPacket *pkt;
>       AVStream *st;
> 
> @@ -178,6 +189,18 @@ static int str_read_packet(AVFormatContext *s,
>           if (channel >= 32)
>               return AVERROR_INVALIDDATA;
> 
> +        sub_mode = sector[0x12];
> +        idx_sect = AV_RL16(&sector[0x1C]);
> +        num_sect = AV_RL16(&sector[0x1E]);
> +
> +        /* compute FPS from sector count @ each new video frame */
> +        if (sub_mode & 0x02 && idx_sect == 0x00) {
> +            int fps = 150 / num_sect;
> +            str->fps_min = FFMIN(str->fps_min, fps);
> +            str->fps_max = FFMAX(str->fps_max, fps);
> +            str->fps_val = FFMIN(MDEC_STR_FPS_MAX, FFMAX(MDEC_STR_FPS_MIN, str->fps_min));
> +        }
> +
>           switch (sector[0x12] & CDXA_TYPE_MASK) {
> 
>           case CDXA_TYPE_DATA:
> @@ -200,7 +223,7 @@ static int str_read_packet(AVFormatContext *s,
>                       st = avformat_new_stream(s, NULL);
>                       if (!st)
>                           return AVERROR(ENOMEM);
> -                    avpriv_set_pts_info(st, 64, 1, 15);
> +                    avpriv_set_pts_info(st, 64, 1, str->fps_val);

This is not a FPS value,
avpriv_set_pts_info() sets the timebase in which timestamps are specified

I dont know psxstr but can you explain what information there is in it
about the video frames ?
is there some sort of information that indicates when a frame is to be
displayed ?
that is like a timestamp ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Elect your leaders based on what they did after the last election, not
based on what they say before an election.