search

polyfloyd / rust-id3

A rust library for reading and writing ID3 metadata
MIT License
245 stars 47 forks source link

WAV/AIFF Write to existing files #69

Closed Serial-ATA closed 2 years ago

Serial-ATA commented 3 years ago

This just makes write_to_wav/write_to_aiff take a File instead of a path

polyfloyd commented 3 years ago

The functions that write to paths should also be able to update existing files. What difference does it make to pass a File instead?

Serial-ATA commented 3 years ago

Sorry, I don't think I worded that right. I meant existing Files. Doing File::open() and using it elsewhere, you can't get its path back. Tag::write_to_path/write_to exist, but you're only able to use paths for wav/aiff.

polyfloyd commented 2 years ago

Wups, apologies for taking so long.

Can you pass a &mut file argument to the reading functions that accept io::Read + io::Seek?

Furthermore, the changes proposed are minor, but breaking. So I can not accept them for a new release. However, I am thinking about what needs to change in a future 1.0 stable release and having predictable read/write functions is definitely in scope :)

Serial-ATA commented 2 years ago

Yeah, reading isn't the issue. This was for consistency's sake, but I just looked and saw Tag::write_to just writes in place. Would it be more appropriate to rename it to Tag::dump_to, or something similar?

Also, from the looks of it, the only way to properly write to a File is to use Tag::write_to_path and let it open it for you. Is that correct? I haven't used this in awhile.

polyfloyd commented 2 years ago

I have reorganized the reading and writing functions to be more consistent and added functions that take &mut File as argument. I intend to release these in v1.0 which should resolve this issue :)

I like your suggestion regarding dump_to. People could mistake write_to for being content aware and overwrite data by accident. Will think about this.