Re: [FFmpeg-devel] [PATCH] avcodec/photocd: allow decoding vlc with errors
Quoting Paul B Mahol (2023-09-28 11:48:14) > On 9/28/23, Anton Khirnov wrote: > > Quoting Paul B Mahol (2023-09-27 21:53:54) > >> Attached. > >> > >> From 6b7e51e2b4facead9c45fb94647c85b58e6fdf22 Mon Sep 17 00:00:00 2001 > >> From: Paul B Mahol > >> Date: Wed, 27 Sep 2023 21:48:49 +0200 > >> Subject: [PATCH] avcodec/photocd: allow decoding vlc with errors > >> > >> Also rename option that no longer works. > >> Also make tables finding more robust. > > > > Why do all these things in a single patch? > > > > I can boost my commits if you prefer that way. Feel welcome. > >> +s->streampos = (AV_RB16(avpkt->data + 2048 + 1536 + 3) + 12) > >> * 2048; > > > > Where do these magic numbers come from? Same below. > > From offset from internal image file bitstream table structure. Please describe that in a comment above that line then. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/photocd: allow decoding vlc with errors
On 9/28/23, Anton Khirnov wrote:
> Quoting Paul B Mahol (2023-09-27 21:53:54)
>> Attached.
>>
>> From 6b7e51e2b4facead9c45fb94647c85b58e6fdf22 Mon Sep 17 00:00:00 2001
>> From: Paul B Mahol
>> Date: Wed, 27 Sep 2023 21:48:49 +0200
>> Subject: [PATCH] avcodec/photocd: allow decoding vlc with errors
>>
>> Also rename option that no longer works.
>> Also make tables finding more robust.
>
> Why do all these things in a single patch?
>
I can boost my commits if you prefer that way.
> And why are there no tests for this decoder?
Its complicated.
>
>> Signed-off-by: Paul B Mahol
>> ---
>> libavcodec/photocd.c | 7 +++
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/photocd.c b/libavcodec/photocd.c
>> index 07e8d460bd..a9e42bb86a 100644
>> --- a/libavcodec/photocd.c
>> +++ b/libavcodec/photocd.c
>> @@ -277,8 +277,6 @@ static av_noinline int decode_huff(AVCodecContext
>> *avctx, AVFrame *frame,
>> if (get_bits_left() <= 0)
>> return AVERROR_INVALIDDATA;
>> m = get_vlc2(, s->vlc[idx].table, s->vlc[idx].bits, 2);
>> -if (m < 0)
>> -return AVERROR_INVALIDDATA;
>
> Shouldn't this error be at least logged? Perhaps it should still fail on
> AV_EF_EXPLODE.
>
>> m = sign_extend(m, 8);
>> data[x] = av_clip_uint8(data[x] + m);
>> }
>> @@ -389,6 +387,7 @@ static int photocd_decode_frame(AVCodecContext *avctx,
>> AVFrame *p,
>> return AVERROR_INVALIDDATA;
>>
>> if (s->resolution == 4) {
>> +s->streampos = (AV_RB16(avpkt->data + 2048 + 1536 + 3) + 12)
>> * 2048;
>
> Where do these magic numbers come from? Same below.
>From offset from internal image file bitstream table structure.
>
> --
> Anton Khirnov
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/photocd: allow decoding vlc with errors
Quoting Paul B Mahol (2023-09-27 21:53:54)
> Attached.
>
> From 6b7e51e2b4facead9c45fb94647c85b58e6fdf22 Mon Sep 17 00:00:00 2001
> From: Paul B Mahol
> Date: Wed, 27 Sep 2023 21:48:49 +0200
> Subject: [PATCH] avcodec/photocd: allow decoding vlc with errors
>
> Also rename option that no longer works.
> Also make tables finding more robust.
Why do all these things in a single patch?
And why are there no tests for this decoder?
> Signed-off-by: Paul B Mahol
> ---
> libavcodec/photocd.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/photocd.c b/libavcodec/photocd.c
> index 07e8d460bd..a9e42bb86a 100644
> --- a/libavcodec/photocd.c
> +++ b/libavcodec/photocd.c
> @@ -277,8 +277,6 @@ static av_noinline int decode_huff(AVCodecContext *avctx,
> AVFrame *frame,
> if (get_bits_left() <= 0)
> return AVERROR_INVALIDDATA;
> m = get_vlc2(, s->vlc[idx].table, s->vlc[idx].bits, 2);
> -if (m < 0)
> -return AVERROR_INVALIDDATA;
Shouldn't this error be at least logged? Perhaps it should still fail on
AV_EF_EXPLODE.
> m = sign_extend(m, 8);
> data[x] = av_clip_uint8(data[x] + m);
> }
> @@ -389,6 +387,7 @@ static int photocd_decode_frame(AVCodecContext *avctx,
> AVFrame *p,
> return AVERROR_INVALIDDATA;
>
> if (s->resolution == 4) {
> +s->streampos = (AV_RB16(avpkt->data + 2048 + 1536 + 3) + 12) *
> 2048;
Where do these magic numbers come from? Same below.
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
