MAD_ERROR_BADHUFFDATA before MAD_ERROR_BUFLEN

[Radek Bartoň-3]

Hello.

I tried to incorporate Madlld low-level API demonstration code into my MP3
player, but I'm experiencing a weird problem. When data stored by  
mad_stream_buffer to MAD are going to run out MAD_ERROR_BADHUFFDATA is
generated right before MAD_ERROR_BUFLEN. Then when new data are sent to MAD
MAD_ERROR_BADCRC and MAD_ERROR_BADDATAPTR occur. Since then no other errors
occur until next data are consumed. This produces slight skip in otherwise
fluent playback. I checked twice that my code which I'm attaching is
equivalent to Madlld's one. Please, if you'll notice a reasong of this
behaviour in my code, let me know since I'm little desperate about this.

Thank you.

--
Bc. Radek Bartoň

Faculty of Information Technology
Brno University of Technology

E-mail: xbarto33@...
Web: http://blackhex.no-ip.org
Jabber: blackhex@...

[mad_problem.cpp]

---

Q_LONG Player::decodeFile(uchar * buffer, const Q_LONG size)
{
  qDebug("decode: %ld", size);
  const Q_LONG MPEG_BUFFER_SIZE = 40 * 1024;
  uchar mpeg_buffer[MPEG_BUFFER_SIZE + MAD_BUFFER_GUARD];
  Q_LONG to_decode = size;
  Q_LONG decoded = 0;

  qDebug("saved: %ld offset: %ld", this->int_saved_size, this->int_saved_start);

  // Store previously buffered PCM samples to output buffer.
  if (this->int_saved_size > 0)
  {
    Q_LONG stored = this->storeSynthToBuffer(buffer, this->int_synth,
      this->int_saved_start, to_decode);
    buffer += stored;
    to_decode -= stored;
    decoded += stored;

    // Consume stored part of buffer.
    this->int_saved_size -= stored;
    this->int_saved_start = (this->int_saved_size > 0) ? this->int_saved_start
      + stored : 0;


    qDebug("BUFFERED: stored: %ld saved: %ld offset: %ld", stored, this->int_saved_size, this->int_saved_start);
  }

  // Decode until desired size is reached.
  while ((to_decode > 0) && !this->int_file.atEnd())
  {
    uchar * guard_start = NULL;

    // It's needed to read next MPEG data from file?
    if ((this->int_stream.buffer == NULL) || (this->int_stream.error ==
      MAD_ERROR_BUFLEN))
    {
      Q_LONG remaining = 0;
      Q_LONG read_size = MPEG_BUFFER_SIZE;
      uchar * read_start = mpeg_buffer;

      // Save undecoded rest of buffer to its beginning.
      if (this->int_stream.next_frame != NULL)
      {
        remaining = this->int_stream.bufend - this->int_stream.next_frame;
        memmove(mpeg_buffer, this->int_stream.next_frame, remaining);
        read_start += remaining;
        read_size -= remaining;
      }

      qDebug("read_size: %ld read_start: %d", read_size, read_start - mpeg_buffer);

      // Read next frame of MPEG data.
      if ((read_size = this->int_file.readBlock((char *) read_start,
        read_size)) < 0)
      {
        qWarning("Error reading from file %s!", this->int_file_name.operator
          const char *());
        return -1;
      }

      // If end of file reached append a few zeros to MPEG data buffer.
      if (this->int_file.atEnd())
      {
        guard_start = read_start + read_size;
        memset(guard_start, 0, MAD_BUFFER_GUARD);
        read_size += MAD_BUFFER_GUARD;
      }

      // Send MPEG data to MAD.
      mad_stream_buffer(&this->int_stream, mpeg_buffer, remaining + read_size);
      this->int_stream.error = MAD_ERROR_NONE;
    }

    // Decode next frame.
    if (mad_frame_decode(&this->int_frame, &this->int_stream) != 0)
    {
      // Is occured error recoverable?
      if (MAD_RECOVERABLE(this->int_stream.error))
      {
        if (this->int_stream.error != MAD_ERROR_LOSTSYNC ||
          this->int_stream.this_frame != guard_start)
        {
          qWarning("Recoverable error during MPEG stream decoding: %x!",
            this->int_stream.error);
        }

        // Decode next frame.
        continue;
      }
      else
      {
        // Out of MPEG frame data?
        if (this->int_stream.error == MAD_ERROR_BUFLEN)
        {
          qDebug("need data");

          // Read next data from file and try to decode aggain.
          continue;
        }
        else
        {
          qWarning("Error decoding MPEG stream: %x!", this->int_stream.error);
          return -1;
        }
      }
    }

    // Synthetize decoded frame to PCM samples.
    mad_synth_frame(&this->int_synth, &this->int_frame);

    qDebug("decoded: %d to_decode: %ld", this->int_synth.pcm.length * 2 * sizeof(short), to_decode);

    // Store decoded PCM samples to output buffer.
    Q_LONG stored = this->storeSynthToBuffer(buffer, this->int_synth, 0,
      to_decode);
    buffer += stored;
    to_decode -= stored;
    decoded += stored;

    // Save rest of a buffer for next call.
    this->int_saved_size = (this->int_synth.pcm.length * 2 * sizeof(short)) -
      stored;
    this->int_saved_start = (this->int_saved_size > 0) ? stored : 0;

    qDebug("NEW: stored: %ld saved: %ld offset: %ld", stored, this->int_saved_size, this->int_saved_start);
  }

  return decoded;
}

Q_LONG Player::storeSynthToBuffer(uchar * buffer, const mad_synth & synth,
  const Q_LONG offset, const Q_LONG size)
{
  Q_LONG stored = 0;
  Q_LONG samples = size / (2 * sizeof(short));
  Q_LONG start = offset / (2 * sizeof(short));
  Q_LONG end = ((start + samples) > synth.pcm.length) ? synth.pcm.length :
    (start + samples);

  // Copy decoded PCM samples to output buffer.
  for (Q_LONG I = start; I < end; ++I)
  {
    // Copy left and right channel.
    for (int J = 0; J < 2; ++J)
    {
      mad_fixed_t mad_sample;
      short sample;

      // Convert MAD fixed point sample to short.
      mad_sample = synth.pcm.samples[J][I];
      sample = (short) (mad_sample >> (MAD_F_FRACBITS - 15));
      if (mad_sample >= MAD_F_ONE)
      {
        sample = SHRT_MAX;
      }
      if (mad_sample <= -MAD_F_ONE)
      {
        sample = -SHRT_MAX;
      }

      // Copy left/right channel sample to output
      memcpy(buffer, &sample, sizeof(sample));
      buffer += sizeof(sample);
      stored += sizeof(sample);
    }
  }

  return stored;
}

---

[Radek Bartoň-3]

I'm reposting Miranda's message which was sent as private and I hope that by
mistake:

There's an obvious problem in your program, I instruct it in the comments:
  /* When error occurred while mad_frame_decode, you need do some processing
*/
 if (MAD_RECOVERABLE(this->int_stream.error))      
      {
        if (this->int_stream.error != MAD_ERROR_LOSTSYNC ||
          this->int_stream.this_frame != guard_start)
        {
          qWarning("Recoverable error during MPEG stream decoding:
%x!",
            this->int_stream.error);
        }
          // Decode next frame.
        continue;
      }
      else          /* The problem occurres here */
      {
        // Out of MPEG frame data?
        if (this->int_stream.error == MAD_ERROR_BUFLEN)  
   /* MAD_ERROR_BUFLEN is a kind of recoverable error, i.e,
MAD_RECOVERABLE(MAD_ERROR_BUFLEN) == 1, so the condition "if
(this->int_stream.error == MAD_ERROR_BUFLEN)" here will never be true, and
you do need it to be true when there's no enough data in this->int_stream, so
you can read new data into buffer from file. */
        {
          qDebug("need data");
            // Read next data from file and try to decode aggain.
          continue;
        }
        else
        {
          qWarning("Error decoding MPEG stream: %x!",
this->int_stream.error);
          return -1;
        }
      }
   
  Hope this will help.
   
  Best regards,
  Miranda
 
       
---------------------------------
 雅虎免费邮箱3.5G容量，20M附件！

[Radek Bartoň-3]

On Sunday 13 of May 2007 01:53:33 Radek Bartoň wrote:

> I'm reposting Miranda's message which was sent as private and I hope that
> by mistake:
>
> There's an obvious problem in your program, I instruct it in the comments:
>   /* When error occurred while mad_frame_decode, you need do some
> processing */
>  if (MAD_RECOVERABLE(this->int_stream.error))      
>       {
>         if (this->int_stream.error != MAD_ERROR_LOSTSYNC ||
>           this->int_stream.this_frame != guard_start)
>         {
>           qWarning("Recoverable error during MPEG stream decoding:
> %x!",
>             this->int_stream.error);
>         }
>           // Decode next frame.
>         continue;
>       }
>       else          /* The problem occurres here */
>       {
>         // Out of MPEG frame data?
>         if (this->int_stream.error == MAD_ERROR_BUFLEN)  
>    /* MAD_ERROR_BUFLEN is a kind of recoverable error, i.e,
> MAD_RECOVERABLE(MAD_ERROR_BUFLEN) == 1, so the condition "if
> (this->int_stream.error == MAD_ERROR_BUFLEN)" here will never be true, and
> you do need it to be true when there's no enough data in this->int_stream,
> so you can read new data into buffer from file. */
>         {
>           qDebug("need data");
>             // Read next data from file and try to decode aggain.
>           continue;
>         }
>         else
>         {
>           qWarning("Error decoding MPEG stream: %x!",
> this->int_stream.error);
>           return -1;
>         }
>       }
>    
>   Hope this will help.
>    
>   Best regards,
>   Miranda
>
>        
> ---------------------------------
>  雅虎免费邮箱3.5G容量，20M附件！

I'm sorry, but it didn't. MAD_RECOVERABLE(MAD_ERROR_BUFLEN) is evaluated as
false with my MAD 0.15b installation. Anyway, if
MAD_RECOVERABLE(MAD_ERROR_BUFLEN) would be true it would work either because
with every recoverable error loop is returned to beginning where
MAD_ERROR_BUFLEN is tested and new data are passed accordingly.

But thank your for your attention to my problem.

--
Bc. Radek Bartoň

Faculty of Information Technology
Brno University of Technology

E-mail: xbarto33@...
Web: http://blackhex.no-ip.org
Jabber: blackhex@...

[Rob Leslie]

On Apr 29, 2007, at 11:01 AM, Radek Bartoň wrote:

> I tried to incorporate Madlld low-level API demonstration code into  
> my MP3 player, but I'm experiencing a weird problem. When data  
> stored by mad_stream_buffer to MAD are going to run out  
> MAD_ERROR_BADHUFFDATA is generated right before MAD_ERROR_BUFLEN.  
> Then when new data are sent to MAD MAD_ERROR_BADCRC and  
> MAD_ERROR_BADDATAPTR occur. Since then no other errors occur until  
> next data are consumed. This produces slight skip in otherwise  
> fluent playback. I checked twice that my code which I'm attaching  
> is equivalent to Madlld's one. Please, if you'll notice a reasong  
> of this behaviour in my code, let me know since I'm little  
> desperate about this.

You should not inspect this->int_stream.error (at line 35) unless  
mad_decode_frame() returns -1, because this field is not otherwise  
reset. I suspect you may be loading new data each time through the  
loop, until another decoding error occurs as a consequence.

--
Rob Leslie
rob@...

[Radek Bartoň-3]

On Monday 21 of May 2007 03:04:46 Rob Leslie wrote:

> You should not inspect this->int_stream.error (at line 35) unless
> mad_decode_frame() returns -1, because this field is not otherwise
> reset. I suspect you may be loading new data each time through the
> loop, until another decoding error occurs as a consequence.

If I understand your advise correctly, you are saying that I should move code
reading new data (lines 35-73) right after MAD_ERROR_BUFLEN detections to
line 97? I tried that but result is the same. Moreover reading code is
executed first time due fullfilled condition this->int_stream.buffer == NULL
and it resets this->int_stream.error to MAD_ERROR_NONE at line 72 each time
so next evocation of this code would be due previous MAD_ERROR_BUFLEN and it
should be called only once for each MAD_ERROR_BUFLEN error.

The only thing I know about problem is that MAD_ERROR_BADHUFFDATA or any other
errors are returned before MAD_ERROR_BUFLEN and this seems like MAD won't
recognize that it hasn't complete frame at the end of buffered data, tries to
decode it with these errors and then returns MAD_ERROR_BUFLEN. Then after
passing new block of data is first frame in buffer of course incomplete too
and MAD gets unsynchronized with errors.

--
Bc. Radek Bartoň

Faculty of Information Technology
Brno University of Technology

E-mail: xbarto33@...
Web: http://blackhex.no-ip.org
Jabber: blackhex@...

[Rob Leslie]

> The only thing I know about problem is that MAD_ERROR_BADHUFFDATA  
> or any other errors are returned before MAD_ERROR_BUFLEN and this  
> seems like MAD won't recognize that it hasn't complete frame at the  
> end of buffered data, tries to decode it with these errors and then  
> returns MAD_ERROR_BUFLEN. Then after passing new block of data is  
> first frame in buffer of course incomplete too and MAD gets  
> unsynchronized with errors.

I just realized that your mpeg_buffer is on the stack. MAD doesn't  
copy this buffer; it keeps pointers to it. Unless you're decoding the  
entire file in one go, this is very likely a source of corruption and  
consequent decoding errors.

Try moving line 5 outside the function, to file scope.

--
Rob Leslie
rob@...

[Radek Bartoň-3]

On Tuesday 22 of May 2007 21:56:58 Rob Leslie wrote:

> I just realized that your mpeg_buffer is on the stack. MAD doesn't
> copy this buffer; it keeps pointers to it. Unless you're decoding the
> entire file in one go, this is very likely a source of corruption and
> consequent decoding errors.
>
> Try moving line 5 outside the function, to file scope.

That's it. Thank you very much. I should realize such obvious mistake but
sometimes I can't see the forest for trees :-).

--
Bc. Radek Bartoň

Faculty of Information Technology
Brno University of Technology

E-mail: xbarto33@...
Web: http://blackhex.no-ip.org
Jabber: blackhex@...