search

sannies / mp4parser

A Java API to read, write and create MP4 files
Apache License 2.0
2.76k stars 566 forks source link

Removing aspectj dependency #59

Open aldenml opened 9 years ago

aldenml commented 9 years ago

Hi @sannies, I would like to discuss the possibility of remove the aspectj dependency.

I underestand the logic behind RequiresParseDetailAspect, but it seems to me that this could be implemented inside the ParserImpl. It would not be as lazy as possible but at the same time it would be more predictable.

I can go ahead with a PR, but I need to know if there is any other use case that I'm not aware of.

This removal would be very beneficial for keeping the library lightweight especially in android.

sannies commented 9 years ago

Hi, this is a good point. I don't use a lot of the whole aspectJ library in the end. It's just this tiny pointcut which effectively doesn't do much more than adding a single line of code (parseDetails() with some guard statement) as the first few lines of each affected method. Would you do it by hand? Or would you introduce some other form of magic? Why do I need the aspectJ runtime jar at all? Do I need it? Best Regards, Sebastian

aldenml commented 9 years ago

I would do it by hand, similar to this: https://github.com/frostwire/frostwire-common/blob/master/core/com/frostwire/util/MP4Muxer.java#L211 but adding the logic from RequiresParseDetailAspect.

As long as you use this pointcut (and the maven aspectj plugin), the aspectJ runtime jar is needed, since the bytecode is changed to be AOP aware.

I think my next step is develop the change and send you the PR for more analysis.

Regards

sannies commented 9 years ago

Just thinking out loud: I always wanted to have interfaces for all boxes so that the 4 char code directly corresponds to an interface. This together with Java Proxy might retain a very similar functionality.

aldenml commented 9 years ago

Any advantage of having an interface per code? Asking in case I'm missing something.

sannies commented 9 years ago

When you generate MP4 as for example in the MP4Builder classes you get a Box structure that contains an anonymous implemtation of the mdat box for example. If you now post process this structure you can only identiy the mdat by 4cc but not by interface. That can be a bit awkward.

2015-03-19 21:33 GMT+02:00 Alden Torres notifications@github.com:

Any advantage of having an interface per code? Asking in case I'm missing something.

— Reply to this email directly or view it on GitHub https://github.com/sannies/mp4parser/issues/59#issuecomment-83733021.

aldenml commented 9 years ago

I see, well, still preparing the PR to discuss in the details.

For now, I'm stuck at CencFileRoundtripTest. It seems that there is a subtle detail in the way parseDetails is executed (order wise).

aldenml commented 9 years ago

I want to add, that if you ever take a look at robovm, I think mp4parser could be used inside iOS, but having aspectj as a dependency could be an obstacle (didn't try it).