dhowden / tag

ID3, MP4 and OGG/FLAC metadata parsing in Go
BSD 2-Clause "Simplified" License
558 stars 72 forks source link

If a mp3 file has no tags at all, the go library reports no tags found #98

Closed pfarrell51 closed 1 year ago

pfarrell51 commented 1 year ago

Its rare, nearly all mp3 files have some tags, but if there are none, the tag.ReadFrom (file) will write out

error reading file: no tags found

without returning an error

"github.com/dhowden/tag" m, err := tag.ReadFrom(file)

If I build the 'tag' executable, and run it on the file, it does give a reasonable error message

tag /home/pfarrell/whome/Music/mp3/CopyHitsForCar/Sweet_Darlin\;\ Heart.mp3 error reading file: no tags found

but the go library does not return an error.

Interesting, the message does not have a /n at the end, so its formatted like most fmt.Errorf strings

dhowden commented 1 year ago

Not quite sure I understand the issue. Can you post an example file and some code so that we can reproduce your issue?

Note: the library function ReadFrom returns the error (https://github.com/dhowden/tag/blob/master/tag.go#L25 ErrNoTagsFound) here: https://github.com/dhowden/tag/blob/master/tag.go#L61.

The tag tool is printing the returned error (in your case ErrNoTagsFound) with some context and a trailing newline here: https://github.com/dhowden/tag/blob/master/cmd/tag/main.go#L48.

If you're using the library you should get back ErrNoTagsFound, not the extended form printed out by the tag tool.

pfarrell51 commented 1 year ago

It takes a special file to duplicate it. I have several, but they contain copyrighted music so I do not want to post them. The file is an MP3 file with no tags in it. I don't know how it was created.

If you run the file against the source built from the github link, you get the error message displayed, it is not trapped

./bugs Sweet_Darlin_Heart.mp3 no tags found

Run it thru your cmd/tag works as expected, gives a complete error $ tag -raw Sweet_Darlin_Heart.mp3 error reading file: no tags found

I can share a file with the developers with the understanding that no intention is made to violate the copyright.

This program will exercise the issue.

The issue is that the library does not return the error, but instead it writes the error message directly to stdout.

Source code on github at https://github.com/pfarrell51/gows/blob/master/music/tagtool/bugs/main.go This is a simple, 35 line program with error traps. Under 5 lines actually reproducing the issue

Send me an email and I'll give you a mp3 file that breaks it

dhowden commented 1 year ago

I wrote a simple program to try this out:

package main

import (
    "flag"
    "log"
    "os"

    "github.com/dhowden/tag"
)

var path = flag.String("path", "", "`path` to file to check")

func main() {
    flag.Parse()

    f, err := os.Open(*path)
    if err != nil {
        log.Fatalf("Could not open: %v", err)
    }

    m, err := tag.ReadFrom(f)
    if err != nil {
        log.Fatalf("Error from ReadFrom: %v", err)
    }
    log.Printf("Tags: %v", m)
}

When run it outputs:

$ ./check-single -path example.mp3
2023/03/15 19:22:44 Error from ReadFrom: no tags found

Not sure what you mean here:

The issue is that the library does not return the error, but instead it writes the error message directly to stdout.

I only get the output from the log.Fatalf("Error from ReadFrom: %v", err) statement, I don't see two no tags found messages (one written out by the ReadFrom, and one from the log stmt).

At this point I realised I missed that you had also posted an example program (thanks - always easier to reproduce with an example!), so also tried that (with some slight modifications to concentrate on the idea of testing if ReadFrom writes out the error to stdout instead of returning it):

package main

import (
    "fmt"
    "os"

    "github.com/dhowden/tag"
)

func main() {
    if len(os.Args) == 1 {
        fmt.Println("Usage, bugs [filespec]")
        return
    }
    arg := os.Args[1]
    if arg == "" {
        fmt.Println("PIB, input path empty")
        return
    }
    file, err := os.Open(arg)
    if err != nil {
        fmt.Printf("err : %v %s\n", err, arg)
        return
    }
    defer file.Close()

    m, err := tag.ReadFrom(file)
    if err != nil {
        // Deliberately don't print anything.  If the lilbrary is printing
        // the error we should see it.
        // fmt.Printf("%v", err)
        return
    }
    if m == nil {
        fmt.Printf("tag.ReadFrom (file) turned nil but no error for %s\n", arg)
    }
    fmt.Println("Got to the end") // Print this if we get to the end
}

With this program, I get no output from the example mp3 file which also seems to suggest that ReadFrom is passing the error back, not writing it out to stdout:

$ ./bugs sample.mp3

Can you check that you are using a clean checkout of the library?

pfarrell51 commented 1 year ago

I deleted all the source in my go tree for dhowden and then rebuilt and rerun my test program I ran it against the example file that I sent you a link to my dropbox. I have several files that will fail, but its only about 3 files out of 6,000

I put in the slight changes you had. // Deliberately don't print anything. If the lilbrary is printing // the error we should see it. //fmt.Printf("%v", err)

The library is not printing it with Printf commented out. My code will print the error if the line is active. fmt.Printf("%v", err)

So looks like programmer error (mine) not library bug (yours)

Thanks for helping me debug this.

Thanks Pat

pfarrell51 commented 1 year ago

No issue here. User error in calling library.