diff mbox series

[FFmpeg-devel,v2,2/7] avformat/apm: prepare extradata handling for muxer

Message ID 20200610233155.1050210-3-zane@zanevaniperen.com
State Superseded
Headers show
Series adpcm_ima_apm encoder + apm muxer
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate fail Make fate failed

Commit Message

Zane van Iperen June 10, 2020, 11:32 p.m. UTC
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/Makefile |   2 +-
 libavformat/apm.c    | 130 +++++++++++++++++++++++--------------------
 2 files changed, 70 insertions(+), 62 deletions(-)

Comments

Andreas Rheinhardt June 11, 2020, 2:12 a.m. UTC | #1
Zane van Iperen:
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavformat/Makefile |   2 +-
>  libavformat/apm.c    | 130 +++++++++++++++++++++++--------------------
>  2 files changed, 70 insertions(+), 62 deletions(-)
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 0658fa3710..bb09dc6563 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -93,7 +93,7 @@ OBJS-$(CONFIG_AMRWB_DEMUXER)             += amr.o
>  OBJS-$(CONFIG_ANM_DEMUXER)               += anm.o
>  OBJS-$(CONFIG_APC_DEMUXER)               += apc.o
>  OBJS-$(CONFIG_APE_DEMUXER)               += ape.o apetag.o img2.o
> -OBJS-$(CONFIG_APM_DEMUXER)               += apm.o riffdec.o
> +OBJS-$(CONFIG_APM_DEMUXER)               += apm.o
>  OBJS-$(CONFIG_APNG_DEMUXER)              += apngdec.o
>  OBJS-$(CONFIG_APNG_MUXER)                += apngenc.o
>  OBJS-$(CONFIG_APTX_DEMUXER)              += aptxdec.o rawdec.o
> diff --git a/libavformat/apm.c b/libavformat/apm.c
> index dc59c16562..57d23200b0 100644
> --- a/libavformat/apm.c
> +++ b/libavformat/apm.c
> @@ -21,12 +21,13 @@
>   */
>  #include "avformat.h"
>  #include "internal.h"
> -#include "riff.h"
>  #include "libavutil/internal.h"
>  #include "libavutil/intreadwrite.h"
>  
> -#define APM_FILE_HEADER_SIZE    20
> -#define APM_VS12_CHUNK_SIZE     76
> +#define APM_FILE_HEADER_SIZE    18
> +#define APM_FILE_EXTRADATA_SIZE 80
> +#define APM_EXTRADATA_SIZE      28
> +
>  #define APM_MAX_READ_SIZE       4096
>  
>  #define APM_TAG_CODEC           0x2000
> @@ -43,34 +44,37 @@ typedef struct APMState {
>      int32_t     saved_l;
>  } APMState;
>  
> -typedef struct APMVS12Chunk {
> +typedef struct APMExtraData {
>      uint32_t    magic;
>      uint32_t    file_size;
>      uint32_t    data_size;
>      uint32_t    unk1;
>      uint32_t    unk2;
>      APMState    state;
> -    uint32_t    pad[7];
> -} APMVS12Chunk;
> +    uint32_t    unk3[7];
> +    uint32_t    data;
> +} APMExtraData;
>  
> -static void apm_parse_vs12(APMVS12Chunk *vs12, const uint8_t *buf)
> +static void apm_parse_extradata(APMExtraData *ed, const uint8_t *buf)
>  {
> -    vs12->magic                 = AV_RL32(buf + 0);
> -    vs12->file_size             = AV_RL32(buf + 4);
> -    vs12->data_size             = AV_RL32(buf + 8);
> -    vs12->unk1                  = AV_RL32(buf + 12);
> -    vs12->unk2                  = AV_RL32(buf + 16);
> -
> -    vs12->state.has_saved       = AV_RL32(buf + 20);
> -    vs12->state.predictor_r     = AV_RL32(buf + 24);
> -    vs12->state.step_index_r    = AV_RL32(buf + 28);
> -    vs12->state.saved_r         = AV_RL32(buf + 32);
> -    vs12->state.predictor_l     = AV_RL32(buf + 36);
> -    vs12->state.step_index_l    = AV_RL32(buf + 40);
> -    vs12->state.saved_l         = AV_RL32(buf + 44);
> -
> -    for (int i = 0; i < FF_ARRAY_ELEMS(vs12->pad); i++)
> -        vs12->pad[i]            = AV_RL32(buf + 48 + (i * 4));
> +    ed->magic                 = AV_RL32(buf + 0);
> +    ed->file_size             = AV_RL32(buf + 4);
> +    ed->data_size             = AV_RL32(buf + 8);
> +    ed->unk1                  = AV_RL32(buf + 12);
> +    ed->unk2                  = AV_RL32(buf + 16);
> +
> +    ed->state.has_saved       = AV_RL32(buf + 20);
> +    ed->state.predictor_r     = AV_RL32(buf + 24);
> +    ed->state.step_index_r    = AV_RL32(buf + 28);
> +    ed->state.saved_r         = AV_RL32(buf + 32);
> +    ed->state.predictor_l     = AV_RL32(buf + 36);
> +    ed->state.step_index_l    = AV_RL32(buf + 40);
> +    ed->state.saved_l         = AV_RL32(buf + 44);
> +
> +    for (int i = 0; i < FF_ARRAY_ELEMS(ed->unk3); i++)
> +        ed->unk3[i]           = AV_RL32(buf + 48 + (i * 4));
> +
> +    ed->data                  = AV_RL32(buf + 76);
>  }
>  
>  static int apm_probe(const AVProbeData *p)
> @@ -94,71 +98,75 @@ static int apm_read_header(AVFormatContext *s)
>  {
>      int64_t ret;
>      AVStream *st;
> -    APMVS12Chunk vs12;
> -    uint8_t buf[APM_VS12_CHUNK_SIZE];
> +    APMExtraData extradata;
> +    AVCodecParameters *par;
> +    uint8_t buf[APM_FILE_EXTRADATA_SIZE];
>  
>      if (!(st = avformat_new_stream(s, NULL)))
>          return AVERROR(ENOMEM);
>  
> -    /* The header starts with a WAVEFORMATEX */
> -    if ((ret = ff_get_wav_header(s, s->pb, st->codecpar, APM_FILE_HEADER_SIZE, 0)) < 0)
> -        return ret;
> -
> -    if (st->codecpar->bits_per_coded_sample != 4)
> +    /*
> +     * This is 98% a WAVEFORMATEX, but there's something screwy with the extradata
> +     * that ff_get_wav_header() can't (and shouldn't) handle properly.
> +     */
> +    if (avio_rl16(s->pb) != APM_TAG_CODEC)
>          return AVERROR_INVALIDDATA;
>  
> -    if (st->codecpar->codec_tag != APM_TAG_CODEC)
> +    par = st->codecpar;
> +    par->channels              = avio_rl16(s->pb);
> +    par->sample_rate           = avio_rl32(s->pb);
> +    par->bit_rate              = avio_rl32(s->pb) * 8;
> +    par->block_align           = avio_rl16(s->pb);
> +    par->bits_per_coded_sample = avio_rl16(s->pb);
> +
> +    if (avio_rl16(s->pb) != APM_FILE_EXTRADATA_SIZE)
>          return AVERROR_INVALIDDATA;
>  
> -    /* ff_get_wav_header() does most of the work, but we need to fix a few things. */
> -    st->codecpar->codec_id              = AV_CODEC_ID_ADPCM_IMA_APM;
> -    st->codecpar->codec_tag             = 0;
> +    if (par->bits_per_coded_sample != 4)
> +        return AVERROR_INVALIDDATA;
>  
> -    if (st->codecpar->channels == 2)
> -        st->codecpar->channel_layout    = AV_CH_LAYOUT_STEREO;
> -    else if (st->codecpar->channels == 1)
> -        st->codecpar->channel_layout    = AV_CH_LAYOUT_MONO;
> +    if (par->channels == 2)
> +        par->channel_layout    = AV_CH_LAYOUT_STEREO;
> +    else if (par->channels == 1)
> +        par->channel_layout    = AV_CH_LAYOUT_MONO;
>      else
>          return AVERROR_INVALIDDATA;
>  
> -    st->codecpar->format                = AV_SAMPLE_FMT_S16;
> -    st->codecpar->bits_per_raw_sample   = 16;
> -    st->codecpar->bit_rate              = st->codecpar->channels *
> -                                          st->codecpar->sample_rate *
> -                                          st->codecpar->bits_per_coded_sample;
> +    par->codec_type            = AVMEDIA_TYPE_AUDIO;
> +    par->codec_id              = AV_CODEC_ID_ADPCM_IMA_APM;
> +    par->format                = AV_SAMPLE_FMT_S16;
> +    par->bits_per_raw_sample   = 16;
> +
> +    /* Now skip the pad that ruins everything. */
> +    if ((ret = avio_skip(s->pb, 2)) < 0)
> +        return ret;
>  
> -    if ((ret = avio_read(s->pb, buf, APM_VS12_CHUNK_SIZE)) < 0)
> +    if ((ret = avio_read(s->pb, buf, APM_FILE_EXTRADATA_SIZE)) < 0)
>          return ret;
> -    else if (ret != APM_VS12_CHUNK_SIZE)
> +    else if (ret != APM_FILE_EXTRADATA_SIZE)
>          return AVERROR(EIO);
>  
> -    apm_parse_vs12(&vs12, buf);
> +    apm_parse_extradata(&extradata, buf);
>  
> -    if (vs12.magic != APM_TAG_VS12) {
> +    if (extradata.magic != APM_TAG_VS12 || extradata.data != APM_TAG_DATA)
>          return AVERROR_INVALIDDATA;
> -    }
>  
> -    if (vs12.state.has_saved) {
> +    if (extradata.state.has_saved) {
>          avpriv_request_sample(s, "Saved Samples");
>          return AVERROR_PATCHWELCOME;
>      }
>  
> -    if (avio_rl32(s->pb) != APM_TAG_DATA)
> -        return AVERROR_INVALIDDATA;
> -
> -    if ((ret = ff_alloc_extradata(st->codecpar, 16)) < 0)
> +    if ((ret = ff_alloc_extradata(par, APM_EXTRADATA_SIZE)) < 0)
>          return ret;
>  
> -    AV_WL32(st->codecpar->extradata +  0, vs12.state.predictor_l);
> -    AV_WL32(st->codecpar->extradata +  4, vs12.state.step_index_l);
> -    AV_WL32(st->codecpar->extradata +  8, vs12.state.predictor_r);
> -    AV_WL32(st->codecpar->extradata + 12, vs12.state.step_index_r);
> +    /* Use the entire state as extradata. */
> +    memcpy(par->extradata, buf + 20, APM_EXTRADATA_SIZE);
>  
> -    avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
> +    avpriv_set_pts_info(st, 64, 1, par->sample_rate);
>      st->start_time  = 0;
> -    st->duration    = vs12.data_size *
> -                      (8 / st->codecpar->bits_per_coded_sample) /
> -                      st->codecpar->channels;
> +    st->duration    = extradata.data_size *
> +                      (8 / par->bits_per_coded_sample) /
> +                      par->channels;
>      return 0;
>  }
>  
> 
So in this and the next patch you change the format of the extradata.
This patch here breaks fate (every patch of a patchset is supposed to
pass fate, not only the last one) which is unacceptable. But there is a
more serious problem: Updating the extradata stuff affects more than one
library, namely libavcodec and libavformat. And it is allowed to update
libavcodec alone (as long as the major version doesn't change) without
updating libavformat. So one could come into a situation where one uses
a libavformat exporting the old extradata format and libavcodec only
accepting the new one. A solution for this would be to accept both
formats in libavcodec (they can easily be distinguished by the extradata
size). The patch for libavcodec would have to go in first. That way fate
wouldn't need any update.

Furthermore, why are you parsing (i.e. copying around) the unk1-3 (I
guess it means "unknown"?) when you don't use it at all? And given that
you now simply use the extradata as it has been read, it might be better
to use ff_get_extradata directly.

- Andreas
Zane van Iperen June 11, 2020, 6:37 a.m. UTC | #2
On Thu, 11 Jun 2020 04:12:46 +0200
"Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote:

> So in this and the next patch you change the format of the extradata.
> This patch here breaks fate (every patch of a patchset is supposed to
> pass fate, not only the last one) which is unacceptable.

It's fixed immediately in the next patch. Wanted to keep the
avcodec/avformat changes separate. Is a moot point anyway.

> But there is
> a more serious problem: Updating the extradata stuff affects more
> than one library, namely libavcodec and libavformat. And it is
> allowed to update libavcodec alone (as long as the major version
> doesn't change) without updating libavformat. So one could come into
> a situation where one uses a libavformat exporting the old extradata
> format and libavcodec only accepting the new one. A solution for this
> would be to accept both formats in libavcodec (they can easily be
> distinguished by the extradata size). The patch for libavcodec would
> have to go in first. That way fate wouldn't need any update.

This is a *very* fringe format, so risk of breakage is virtually none,
but I see the point regardless. Have changed it to this:

if (avctx->extradata) {
    if (avctx->extradata_size >= 28) {
        /* new */
    } else if (avctx->extradata_size >= 16) {
        /* old */
    }
}

> 
> Furthermore, why are you parsing (i.e. copying around) the unk1-3 (I
> guess it means "unknown"?) when you don't use it at all?

Honestly, just so there's some documentation of the format somewhere.
I'd be happy to remove them from apm_parse_extradata(), but I'd like to
keep them in the struct, just to show the layout.

> And given
> that you now simply use the extradata as it has been read, it might
> be better to use ff_get_extradata directly.
> 

There's actually two "extradata"s here. The "file" extradata, and the
"codec" extradata.

The "file" extradata is the extradata field of the WAVEFORMATEX (the
APMExtraData structure). This contains info which is obviously not
meant/useful for the codec.

A subsection of this is the APMState structure, which is what I give to
the codec.

I didn't use ff_get_extradata() beacuse I don't want to give the entire
APMExtraData structure to the codec. As I use fields both before and
after the APMState, I thought it better to read the entire thing, then
memcpy() out the needed subsection.


And from your other email:

> > +    case AV_CODEC_ID_ADPCM_IMA_APM:
> > +        avctx->frame_size = BLKSIZE * 2 / avctx->channels;
> > +        avctx->block_align = BLKSIZE;
> > +
> > +        if (!(avctx->extradata = av_mallocz(28)))  
> 
> Missing padding. And zero-initializing the extradata is really enough?

Have added the padding. And yep, it's the APMState structure, so zero'd
everything is fine.
diff mbox series

Patch

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 0658fa3710..bb09dc6563 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -93,7 +93,7 @@  OBJS-$(CONFIG_AMRWB_DEMUXER)             += amr.o
 OBJS-$(CONFIG_ANM_DEMUXER)               += anm.o
 OBJS-$(CONFIG_APC_DEMUXER)               += apc.o
 OBJS-$(CONFIG_APE_DEMUXER)               += ape.o apetag.o img2.o
-OBJS-$(CONFIG_APM_DEMUXER)               += apm.o riffdec.o
+OBJS-$(CONFIG_APM_DEMUXER)               += apm.o
 OBJS-$(CONFIG_APNG_DEMUXER)              += apngdec.o
 OBJS-$(CONFIG_APNG_MUXER)                += apngenc.o
 OBJS-$(CONFIG_APTX_DEMUXER)              += aptxdec.o rawdec.o
diff --git a/libavformat/apm.c b/libavformat/apm.c
index dc59c16562..57d23200b0 100644
--- a/libavformat/apm.c
+++ b/libavformat/apm.c
@@ -21,12 +21,13 @@ 
  */
 #include "avformat.h"
 #include "internal.h"
-#include "riff.h"
 #include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 
-#define APM_FILE_HEADER_SIZE    20
-#define APM_VS12_CHUNK_SIZE     76
+#define APM_FILE_HEADER_SIZE    18
+#define APM_FILE_EXTRADATA_SIZE 80
+#define APM_EXTRADATA_SIZE      28
+
 #define APM_MAX_READ_SIZE       4096
 
 #define APM_TAG_CODEC           0x2000
@@ -43,34 +44,37 @@  typedef struct APMState {
     int32_t     saved_l;
 } APMState;
 
-typedef struct APMVS12Chunk {
+typedef struct APMExtraData {
     uint32_t    magic;
     uint32_t    file_size;
     uint32_t    data_size;
     uint32_t    unk1;
     uint32_t    unk2;
     APMState    state;
-    uint32_t    pad[7];
-} APMVS12Chunk;
+    uint32_t    unk3[7];
+    uint32_t    data;
+} APMExtraData;
 
-static void apm_parse_vs12(APMVS12Chunk *vs12, const uint8_t *buf)
+static void apm_parse_extradata(APMExtraData *ed, const uint8_t *buf)
 {
-    vs12->magic                 = AV_RL32(buf + 0);
-    vs12->file_size             = AV_RL32(buf + 4);
-    vs12->data_size             = AV_RL32(buf + 8);
-    vs12->unk1                  = AV_RL32(buf + 12);
-    vs12->unk2                  = AV_RL32(buf + 16);
-
-    vs12->state.has_saved       = AV_RL32(buf + 20);
-    vs12->state.predictor_r     = AV_RL32(buf + 24);
-    vs12->state.step_index_r    = AV_RL32(buf + 28);
-    vs12->state.saved_r         = AV_RL32(buf + 32);
-    vs12->state.predictor_l     = AV_RL32(buf + 36);
-    vs12->state.step_index_l    = AV_RL32(buf + 40);
-    vs12->state.saved_l         = AV_RL32(buf + 44);
-
-    for (int i = 0; i < FF_ARRAY_ELEMS(vs12->pad); i++)
-        vs12->pad[i]            = AV_RL32(buf + 48 + (i * 4));
+    ed->magic                 = AV_RL32(buf + 0);
+    ed->file_size             = AV_RL32(buf + 4);
+    ed->data_size             = AV_RL32(buf + 8);
+    ed->unk1                  = AV_RL32(buf + 12);
+    ed->unk2                  = AV_RL32(buf + 16);
+
+    ed->state.has_saved       = AV_RL32(buf + 20);
+    ed->state.predictor_r     = AV_RL32(buf + 24);
+    ed->state.step_index_r    = AV_RL32(buf + 28);
+    ed->state.saved_r         = AV_RL32(buf + 32);
+    ed->state.predictor_l     = AV_RL32(buf + 36);
+    ed->state.step_index_l    = AV_RL32(buf + 40);
+    ed->state.saved_l         = AV_RL32(buf + 44);
+
+    for (int i = 0; i < FF_ARRAY_ELEMS(ed->unk3); i++)
+        ed->unk3[i]           = AV_RL32(buf + 48 + (i * 4));
+
+    ed->data                  = AV_RL32(buf + 76);
 }
 
 static int apm_probe(const AVProbeData *p)
@@ -94,71 +98,75 @@  static int apm_read_header(AVFormatContext *s)
 {
     int64_t ret;
     AVStream *st;
-    APMVS12Chunk vs12;
-    uint8_t buf[APM_VS12_CHUNK_SIZE];
+    APMExtraData extradata;
+    AVCodecParameters *par;
+    uint8_t buf[APM_FILE_EXTRADATA_SIZE];
 
     if (!(st = avformat_new_stream(s, NULL)))
         return AVERROR(ENOMEM);
 
-    /* The header starts with a WAVEFORMATEX */
-    if ((ret = ff_get_wav_header(s, s->pb, st->codecpar, APM_FILE_HEADER_SIZE, 0)) < 0)
-        return ret;
-
-    if (st->codecpar->bits_per_coded_sample != 4)
+    /*
+     * This is 98% a WAVEFORMATEX, but there's something screwy with the extradata
+     * that ff_get_wav_header() can't (and shouldn't) handle properly.
+     */
+    if (avio_rl16(s->pb) != APM_TAG_CODEC)
         return AVERROR_INVALIDDATA;
 
-    if (st->codecpar->codec_tag != APM_TAG_CODEC)
+    par = st->codecpar;
+    par->channels              = avio_rl16(s->pb);
+    par->sample_rate           = avio_rl32(s->pb);
+    par->bit_rate              = avio_rl32(s->pb) * 8;
+    par->block_align           = avio_rl16(s->pb);
+    par->bits_per_coded_sample = avio_rl16(s->pb);
+
+    if (avio_rl16(s->pb) != APM_FILE_EXTRADATA_SIZE)
         return AVERROR_INVALIDDATA;
 
-    /* ff_get_wav_header() does most of the work, but we need to fix a few things. */
-    st->codecpar->codec_id              = AV_CODEC_ID_ADPCM_IMA_APM;
-    st->codecpar->codec_tag             = 0;
+    if (par->bits_per_coded_sample != 4)
+        return AVERROR_INVALIDDATA;
 
-    if (st->codecpar->channels == 2)
-        st->codecpar->channel_layout    = AV_CH_LAYOUT_STEREO;
-    else if (st->codecpar->channels == 1)
-        st->codecpar->channel_layout    = AV_CH_LAYOUT_MONO;
+    if (par->channels == 2)
+        par->channel_layout    = AV_CH_LAYOUT_STEREO;
+    else if (par->channels == 1)
+        par->channel_layout    = AV_CH_LAYOUT_MONO;
     else
         return AVERROR_INVALIDDATA;
 
-    st->codecpar->format                = AV_SAMPLE_FMT_S16;
-    st->codecpar->bits_per_raw_sample   = 16;
-    st->codecpar->bit_rate              = st->codecpar->channels *
-                                          st->codecpar->sample_rate *
-                                          st->codecpar->bits_per_coded_sample;
+    par->codec_type            = AVMEDIA_TYPE_AUDIO;
+    par->codec_id              = AV_CODEC_ID_ADPCM_IMA_APM;
+    par->format                = AV_SAMPLE_FMT_S16;
+    par->bits_per_raw_sample   = 16;
+
+    /* Now skip the pad that ruins everything. */
+    if ((ret = avio_skip(s->pb, 2)) < 0)
+        return ret;
 
-    if ((ret = avio_read(s->pb, buf, APM_VS12_CHUNK_SIZE)) < 0)
+    if ((ret = avio_read(s->pb, buf, APM_FILE_EXTRADATA_SIZE)) < 0)
         return ret;
-    else if (ret != APM_VS12_CHUNK_SIZE)
+    else if (ret != APM_FILE_EXTRADATA_SIZE)
         return AVERROR(EIO);
 
-    apm_parse_vs12(&vs12, buf);
+    apm_parse_extradata(&extradata, buf);
 
-    if (vs12.magic != APM_TAG_VS12) {
+    if (extradata.magic != APM_TAG_VS12 || extradata.data != APM_TAG_DATA)
         return AVERROR_INVALIDDATA;
-    }
 
-    if (vs12.state.has_saved) {
+    if (extradata.state.has_saved) {
         avpriv_request_sample(s, "Saved Samples");
         return AVERROR_PATCHWELCOME;
     }
 
-    if (avio_rl32(s->pb) != APM_TAG_DATA)
-        return AVERROR_INVALIDDATA;
-
-    if ((ret = ff_alloc_extradata(st->codecpar, 16)) < 0)
+    if ((ret = ff_alloc_extradata(par, APM_EXTRADATA_SIZE)) < 0)
         return ret;
 
-    AV_WL32(st->codecpar->extradata +  0, vs12.state.predictor_l);
-    AV_WL32(st->codecpar->extradata +  4, vs12.state.step_index_l);
-    AV_WL32(st->codecpar->extradata +  8, vs12.state.predictor_r);
-    AV_WL32(st->codecpar->extradata + 12, vs12.state.step_index_r);
+    /* Use the entire state as extradata. */
+    memcpy(par->extradata, buf + 20, APM_EXTRADATA_SIZE);
 
-    avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
+    avpriv_set_pts_info(st, 64, 1, par->sample_rate);
     st->start_time  = 0;
-    st->duration    = vs12.data_size *
-                      (8 / st->codecpar->bits_per_coded_sample) /
-                      st->codecpar->channels;
+    st->duration    = extradata.data_size *
+                      (8 / par->bits_per_coded_sample) /
+                      par->channels;
     return 0;
 }