| Message ID | 20200214201938.10463-1-onemda@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avcodec: fix pcm zork decoder | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Am Fr., 14. Feb. 2020 um 21:25 Uhr schrieb Paul B Mahol <onemda@gmail.com>: > diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c > index 621a87cf34..48d0e91370 100644 > --- a/libavcodec/codec_desc.c > +++ b/libavcodec/codec_desc.c > @@ -1871,8 +1871,8 @@ static const AVCodecDescriptor codec_descriptors[] = { > { > .id = AV_CODEC_ID_PCM_ZORK, > .type = AVMEDIA_TYPE_AUDIO, > - .name = "pcm_zork", > - .long_name = NULL_IF_CONFIG_SMALL("PCM Zork"), > + .name = "adpcm_zork", > + .long_name = NULL_IF_CONFIG_SMALL("ADPCM Zork"), Didn't we consider this an ABI break in the past? Carl Eugen
On 2/15/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Am Fr., 14. Feb. 2020 um 21:25 Uhr schrieb Paul B Mahol <onemda@gmail.com>: > >> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c >> index 621a87cf34..48d0e91370 100644 >> --- a/libavcodec/codec_desc.c >> +++ b/libavcodec/codec_desc.c >> @@ -1871,8 +1871,8 @@ static const AVCodecDescriptor codec_descriptors[] = >> { >> { >> .id = AV_CODEC_ID_PCM_ZORK, >> .type = AVMEDIA_TYPE_AUDIO, >> - .name = "pcm_zork", >> - .long_name = NULL_IF_CONFIG_SMALL("PCM Zork"), >> + .name = "adpcm_zork", >> + .long_name = NULL_IF_CONFIG_SMALL("ADPCM Zork"), > > Didn't we consider this an ABI break in the past? > I can remove pcm_zork, deprecate PCM_ZORK codec id, and add new ADPCM_ZORK codec id which this decoder will use. > Carl Eugen > _______________________________________________ > 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".
Am Sa., 15. Feb. 2020 um 09:45 Uhr schrieb Paul B Mahol <onemda@gmail.com>: > > On 2/15/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Fr., 14. Feb. 2020 um 21:25 Uhr schrieb Paul B Mahol <onemda@gmail.com>: > > > >> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c > >> index 621a87cf34..48d0e91370 100644 > >> --- a/libavcodec/codec_desc.c > >> +++ b/libavcodec/codec_desc.c > >> @@ -1871,8 +1871,8 @@ static const AVCodecDescriptor codec_descriptors[] = > >> { > >> { > >> .id = AV_CODEC_ID_PCM_ZORK, > >> .type = AVMEDIA_TYPE_AUDIO, > >> - .name = "pcm_zork", > >> - .long_name = NULL_IF_CONFIG_SMALL("PCM Zork"), > >> + .name = "adpcm_zork", > >> + .long_name = NULL_IF_CONFIG_SMALL("ADPCM Zork"), > > > > Didn't we consider this an ABI break in the past? > > I can remove pcm_zork, deprecate PCM_ZORK codec id, and add new > ADPCM_ZORK codec id which this decoder will use. No strong opinion here, perhaps others want to comment... Carl Eugen
On 2/15/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Am Sa., 15. Feb. 2020 um 09:45 Uhr schrieb Paul B Mahol <onemda@gmail.com>: >> >> On 2/15/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > Am Fr., 14. Feb. 2020 um 21:25 Uhr schrieb Paul B Mahol >> > <onemda@gmail.com>: >> > >> >> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c >> >> index 621a87cf34..48d0e91370 100644 >> >> --- a/libavcodec/codec_desc.c >> >> +++ b/libavcodec/codec_desc.c >> >> @@ -1871,8 +1871,8 @@ static const AVCodecDescriptor >> >> codec_descriptors[] = >> >> { >> >> { >> >> .id = AV_CODEC_ID_PCM_ZORK, >> >> .type = AVMEDIA_TYPE_AUDIO, >> >> - .name = "pcm_zork", >> >> - .long_name = NULL_IF_CONFIG_SMALL("PCM Zork"), >> >> + .name = "adpcm_zork", >> >> + .long_name = NULL_IF_CONFIG_SMALL("ADPCM Zork"), >> > >> > Didn't we consider this an ABI break in the past? >> >> I can remove pcm_zork, deprecate PCM_ZORK codec id, and add new >> ADPCM_ZORK codec id which this decoder will use. > > No strong opinion here, perhaps others want to comment... Actually it is more logical so it is done. > > Carl Eugen > _______________________________________________ > 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".
On 2/15/20, Paul B Mahol <onemda@gmail.com> wrote: > On 2/15/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> Am Sa., 15. Feb. 2020 um 09:45 Uhr schrieb Paul B Mahol >> <onemda@gmail.com>: >>> >>> On 2/15/20, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> > Am Fr., 14. Feb. 2020 um 21:25 Uhr schrieb Paul B Mahol >>> > <onemda@gmail.com>: >>> > >>> >> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c >>> >> index 621a87cf34..48d0e91370 100644 >>> >> --- a/libavcodec/codec_desc.c >>> >> +++ b/libavcodec/codec_desc.c >>> >> @@ -1871,8 +1871,8 @@ static const AVCodecDescriptor >>> >> codec_descriptors[] = >>> >> { >>> >> { >>> >> .id = AV_CODEC_ID_PCM_ZORK, >>> >> .type = AVMEDIA_TYPE_AUDIO, >>> >> - .name = "pcm_zork", >>> >> - .long_name = NULL_IF_CONFIG_SMALL("PCM Zork"), >>> >> + .name = "adpcm_zork", >>> >> + .long_name = NULL_IF_CONFIG_SMALL("ADPCM Zork"), >>> > >>> > Didn't we consider this an ABI break in the past? >>> >>> I can remove pcm_zork, deprecate PCM_ZORK codec id, and add new >>> ADPCM_ZORK codec id which this decoder will use. >> >> No strong opinion here, perhaps others want to comment... > > Actually it is more logical so it is done. > Will apply soon.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 7825f2741b..b007b1e31e 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -809,7 +809,6 @@ OBJS-$(CONFIG_PCM_U32LE_DECODER) += pcm.o OBJS-$(CONFIG_PCM_U32LE_ENCODER) += pcm.o OBJS-$(CONFIG_PCM_VIDC_DECODER) += pcm.o OBJS-$(CONFIG_PCM_VIDC_ENCODER) += pcm.o -OBJS-$(CONFIG_PCM_ZORK_DECODER) += pcm.o OBJS-$(CONFIG_ADPCM_4XM_DECODER) += adpcm.o adpcm_data.o OBJS-$(CONFIG_ADPCM_ADX_DECODER) += adxdec.o adx.o @@ -864,6 +863,7 @@ OBJS-$(CONFIG_ADPCM_VIMA_DECODER) += vima.o adpcm_data.o OBJS-$(CONFIG_ADPCM_XA_DECODER) += adpcm.o adpcm_data.o OBJS-$(CONFIG_ADPCM_YAMAHA_DECODER) += adpcm.o adpcm_data.o OBJS-$(CONFIG_ADPCM_YAMAHA_ENCODER) += adpcmenc.o adpcm_data.o +OBJS-$(CONFIG_PCM_ZORK_DECODER) += adpcm.o adpcm_data.o # hardware accelerators OBJS-$(CONFIG_D3D11VA) += dxva2.o diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c index 0cd28431d7..6ede0bbb09 100644 --- a/libavcodec/adpcm.c +++ b/libavcodec/adpcm.c @@ -83,6 +83,10 @@ static const int8_t swf_index_tables[4][16] = { /*5*/ { -1, -1, -1, -1, -1, -1, -1, -1, 1, 2, 4, 6, 8, 10, 13, 16 } }; +static const int8_t zork_index_table[8] = { + -1, -1, -1, 1, 4, 7, 10, 12, +}; + /* end of tables */ typedef struct ADPCMDecodeContext { @@ -154,6 +158,10 @@ static av_cold int adpcm_decode_init(AVCodecContext * avctx) if (avctx->bits_per_coded_sample != 4) return AVERROR_INVALIDDATA; break; + case AV_CODEC_ID_PCM_ZORK: + if (avctx->bits_per_coded_sample != 8) + return AVERROR_INVALIDDATA; + break; default: break; } @@ -416,6 +424,41 @@ static inline int16_t adpcm_mtaf_expand_nibble(ADPCMChannelStatus *c, uint8_t ni return c->predictor; } +static inline int16_t adpcm_zork_expand_nibble(ADPCMChannelStatus *c, uint8_t nibble) +{ + int16_t index = c->step_index; + uint32_t lookup_sample = ff_adpcm_step_table[index]; + int32_t sample = 0; + + if (nibble & 0x40) + sample += lookup_sample; + if (nibble & 0x20) + sample += lookup_sample >> 1; + if (nibble & 0x10) + sample += lookup_sample >> 2; + if (nibble & 0x08) + sample += lookup_sample >> 3; + if (nibble & 0x04) + sample += lookup_sample >> 4; + if (nibble & 0x02) + sample += lookup_sample >> 5; + if (nibble & 0x01) + sample += lookup_sample >> 6; + if (nibble & 0x80) + sample = -sample; + + sample += c->predictor; + sample = av_clip_int16(sample); + + index += zork_index_table[(nibble >> 4) & 7]; + index = av_clip(index, 0, 88); + + c->predictor = sample; + c->step_index = index; + + return sample; +} + static int xa_decode(AVCodecContext *avctx, int16_t *out0, int16_t *out1, const uint8_t *in, ADPCMChannelStatus *left, ADPCMChannelStatus *right, int channels, int sample_offset) @@ -780,6 +823,9 @@ static int get_nb_samples(AVCodecContext *avctx, GetByteContext *gb, case AV_CODEC_ID_ADPCM_PSX: nb_samples = buf_size / (16 * ch) * 28; break; + case AV_CODEC_ID_PCM_ZORK: + nb_samples = buf_size / ch; + break; } /* validate coded sample count */ @@ -1842,6 +1888,19 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data, } } break; + case AV_CODEC_ID_PCM_ZORK: + if (!c->has_status) { + for (channel = 0; channel < avctx->channels; channel++) { + c->status[channel].predictor = 0; + c->status[channel].step_index = 0; + } + c->has_status = 1; + } + for (n = 0; n < nb_samples * avctx->channels; n++) { + int v = bytestream2_get_byteu(&gb); + *samples++ = adpcm_zork_expand_nibble(&c->status[n % avctx->channels], v); + } + break; default: av_assert0(0); // unsupported codec_id should not happen } @@ -1930,3 +1989,4 @@ ADPCM_DECODER(AV_CODEC_ID_ADPCM_THP_LE, sample_fmts_s16p, adpcm_thp_le, ADPCM_DECODER(AV_CODEC_ID_ADPCM_THP, sample_fmts_s16p, adpcm_thp, "ADPCM Nintendo THP"); ADPCM_DECODER(AV_CODEC_ID_ADPCM_XA, sample_fmts_s16p, adpcm_xa, "ADPCM CDROM XA"); ADPCM_DECODER(AV_CODEC_ID_ADPCM_YAMAHA, sample_fmts_s16, adpcm_yamaha, "ADPCM Yamaha"); +ADPCM_DECODER(AV_CODEC_ID_PCM_ZORK, sample_fmts_s16, adpcm_zork, "ADPCM Zork"); diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index 0ad3338f9a..fcb3645ab1 100644 --- a/libavcodec/allcodecs.c +++ b/libavcodec/allcodecs.c @@ -565,7 +565,6 @@ extern AVCodec ff_pcm_u32le_encoder; extern AVCodec ff_pcm_u32le_decoder; extern AVCodec ff_pcm_vidc_encoder; extern AVCodec ff_pcm_vidc_decoder; -extern AVCodec ff_pcm_zork_decoder; /* DPCM codecs */ extern AVCodec ff_gremlin_dpcm_decoder; @@ -629,6 +628,7 @@ extern AVCodec ff_adpcm_vima_decoder; extern AVCodec ff_adpcm_xa_decoder; extern AVCodec ff_adpcm_yamaha_encoder; extern AVCodec ff_adpcm_yamaha_decoder; +extern AVCodec ff_adpcm_zork_decoder; /* subtitles */ extern AVCodec ff_ssa_encoder; diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c index 621a87cf34..48d0e91370 100644 --- a/libavcodec/codec_desc.c +++ b/libavcodec/codec_desc.c @@ -1871,8 +1871,8 @@ static const AVCodecDescriptor codec_descriptors[] = { { .id = AV_CODEC_ID_PCM_ZORK, .type = AVMEDIA_TYPE_AUDIO, - .name = "pcm_zork", - .long_name = NULL_IF_CONFIG_SMALL("PCM Zork"), + .name = "adpcm_zork", + .long_name = NULL_IF_CONFIG_SMALL("ADPCM Zork"), .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY, }, { diff --git a/libavcodec/pcm.c b/libavcodec/pcm.c index 0c4b452b0e..6346510de0 100644 --- a/libavcodec/pcm.c +++ b/libavcodec/pcm.c @@ -491,14 +491,6 @@ static int pcm_decode_frame(AVCodecContext *avctx, void *data, bytestream_get_buffer(&src, samples, n * sample_size); } break; - case AV_CODEC_ID_PCM_ZORK: - for (; n > 0; n--) { - int v = *src++; - if (v < 128) - v = 128 - v; - *samples++ = v; - } - break; case AV_CODEC_ID_PCM_ALAW: case AV_CODEC_ID_PCM_MULAW: case AV_CODEC_ID_PCM_VIDC: @@ -626,7 +618,6 @@ PCM_CODEC (PCM_U24BE, AV_SAMPLE_FMT_S32, pcm_u24be, "PCM unsigned PCM_CODEC (PCM_U24LE, AV_SAMPLE_FMT_S32, pcm_u24le, "PCM unsigned 24-bit little-endian"); PCM_CODEC (PCM_U32BE, AV_SAMPLE_FMT_S32, pcm_u32be, "PCM unsigned 32-bit big-endian"); PCM_CODEC (PCM_U32LE, AV_SAMPLE_FMT_S32, pcm_u32le, "PCM unsigned 32-bit little-endian"); -PCM_DECODER(PCM_ZORK, AV_SAMPLE_FMT_U8, pcm_zork, "PCM Zork"); PCM_CODEC (PCM_S64BE, AV_SAMPLE_FMT_S64, pcm_s64be, "PCM signed 64-bit big-endian"); PCM_CODEC (PCM_S64LE, AV_SAMPLE_FMT_S64, pcm_s64le, "PCM signed 64-bit little-endian"); PCM_CODEC (PCM_VIDC, AV_SAMPLE_FMT_S16, pcm_vidc, "PCM Archimedes VIDC");
Fixes #1939 Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavcodec/Makefile | 2 +- libavcodec/adpcm.c | 60 +++++++++++++++++++++++++++++++++++++++++ libavcodec/allcodecs.c | 2 +- libavcodec/codec_desc.c | 4 +-- libavcodec/pcm.c | 9 ------- 5 files changed, 64 insertions(+), 13 deletions(-)