Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#93 flac (decoder): simplify EOF state and fix MD5 check

open
nobody
None
5
2013-09-29
2012-06-10
eric_wong
No

This patch depends on commit e9a8dcf780f1cbb6a6faf63b7ba28591963c1cbd
(flac: fix invalid memory access w/ optimize_trim) in the "dot" branch.
This patch is also available from my git repo:

git pull git://bogomips.org/sox dot-flac-eof-md5

There's no need to keep track of EOF state ourselves as the FLAC
decoder already keeps track of decoder state us. Instead, we
only need to keep track of whether or not we've seeked for MD5
verification.

To test the ability of the flac handler to detect bad MD5s,
I changed the MD5 checksum of a good FLAC file using the
undocumented "--set-md5sum" option of metaflac(1):

metaflac --set-md5sum d41d8cd98f00b204e9800998ecf8427e foo.flac

This let me decode the entire file without errors, but
ultimately fail the MD5 check.

Discussion

  • eric_wong
    eric_wong
    2012-06-10

    My original patch failed to take into account skipping tracks and incorrectly
    warned about MD5 mismatches when hitting Ctrl-C in play.

    I've squashed the following change into my original patch and repushed
    to "dot-flac-eof-md5" of git://bogomips.org/sox
    The current commit should be 1dedd809d7e095a3398181e25d235573961e20e8

    --- a/src/flac.c
    +++ b/src/flac.c
    @@ -275,7 +275,7 @@ static size_t read_samples(sox_format_t * const ft, sox_sample_t * sampleBuffer,
    static int stop_read(sox_format_t * const ft)
    {
    priv_t * p = (priv_t *)ft->priv;
    - if (!FLAC__stream_decoder_finish(p->decoder) && ! p->seek_done)
    + if (!FLAC__stream_decoder_finish(p->decoder) && ! p->seek_done && FLAC__stream_decoder_get_state(p->decoder) == FLAC__STREAM_DECODER_END_OF_STREAM)
    lsx_warn("decoder MD5 checksum mismatch.");
    FLAC__stream_decoder_delete(p->decoder);

     
  • eric_wong
    eric_wong
    2013-09-29

    I'm pretty sure this can be dropped (as per previous discussion around early 2013 on the dev ML)