Gagravarr / VorbisJava

A library for working with Ogg Vorbis files
Apache License 2.0
126 stars 26 forks source link

Reworked OpusInfo #11

Closed andrm closed 9 years ago

andrm commented 9 years ago

Let me know any further comments.

Gagravarr commented 9 years ago

I'm not sure that OpusInfo is the right place to put all of the logic. If we follow the model in the other formats, it's there to prodive read + write, including high level getters and setters, on everything in the info

Currently, VorbisInfoTool does some of the vorbis equivalents of this, but it means a little duplication between the Info Tool and the Tika parsers

I'd probably therefore lean towards having a new class in the core package, inheriting from a common (new) base class. That base class could implement the logic in OggAudioParser.extractDuration, which could then be used commonly from the Info Tools and the Tika stuff, reducing duplication. We could then have an Opus specific extension to it, with all your logic

Does that make sense a plan? If so, I can refactor the tika bit into a common base class, then perhaps you could push your nifty new bit of logic in the opus one?

Alternate ideas welcome too :)

andrm commented 9 years ago

Yes, that sounds like a plan. No alternate ideas, I would follow your lead as you know the code better and a concept for the tika integration, which is arguably the main use of this code.

I'll withdraw this pull request and wait for the base class.

I'm also interested in your take on multiple streams in one ogg/opus file with regard to extracting this info. Is this important? Are we fine with the current case?

Gagravarr commented 9 years ago

I've add a stub OpusInfoTool, based on the Vorbis one and a new superclass

I've moved the basic statistics calculations from the Tika Parsers and VorbisInfoTool into OggAudioStatistics

It should now (hopefully!) be possible to put your logic into a new OpusStatistics class extending from OggAudioStatistics, and then update OpusInfoTool to use that and output the extra info

Gagravarr commented 9 years ago

In terms of multiple streams in one file, I'm not sure it's too big an issue for pure-audio files. However, it is a real issue for video streams. I'm hoping that Ray Gauss is going to be able to have some time soon to look into this more from the Tika side, especially for video files. That may give some hints on what else is needed for good support.

In the mean time, the big challenge is getting some small multi-stream sample files. I've got a few, including some with skeletons, but not as many as I'd really like. Once we have them, we can then review the info tools, and see what's the best way to get them to handle these files