diff mbox series

[FFmpeg-devel] avcodec: fix pcm zork decoder

Message ID 20200214201938.10463-1-onemda@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] avcodec: fix pcm zork decoder
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Paul B Mahol Feb. 14, 2020, 8:19 p.m. UTC
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(-)

Comments

Carl Eugen Hoyos Feb. 15, 2020, 8:18 a.m. UTC | #1
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
Paul B Mahol Feb. 15, 2020, 8:45 a.m. UTC | #2
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".
Carl Eugen Hoyos Feb. 15, 2020, 9:06 a.m. UTC | #3
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
Paul B Mahol Feb. 15, 2020, 1:15 p.m. UTC | #4
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".
Paul B Mahol Feb. 16, 2020, 9:39 a.m. UTC | #5
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 mbox series

Patch

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");