Add audio url support

[bcoley]

Adding support for audio from a streaming internet radio station, example program included.

[bcoley]

It's been a while since I did much on github, do I need to do anything else or is this one all set for you to merge? Thanks!

[hamoid]

Hi! Thanks, I did ask a question above but maybe you didn't notice, about line 299 :) Actually, click on Files Changed at the top to see the comment in context and be able to reply.

[bcoley]

Sorry, didn't see that, I will take a look.

On Sun, May 6, 2018 at 3:45 AM, Abe Pazos notifications@github.com wrote:

Hi! Thanks, I did ask a question above but maybe you didn't notice, about line 299 :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hamoid/video_export_processing/pull/51#issuecomment-386860826, or mute the thread https://github.com/notifications/unsubscribe-auth/AFE-gfhjGkiAcKdGZp0KMMkfGq3jV-Zyks5tvqoMgaJpZM4TtUZq .

-- Regards, Brett

[bcoley]

Thanks, I am looking at the changed files page and I don't see a comment, maybe I am doing something wrong? The intent of the checks in those setters was to prevent any confusion as to whether they could be used together, that is, callers should setAudioFileName() or setAudioUrl(), and if they attempt to use both, those checks are to prevent that, similar to other checks in the code.

On Sun, May 6, 2018 at 3:45 AM, Abe Pazos notifications@github.com wrote:

Hi! Thanks, I did ask a question above but maybe you didn't notice, about line 299 :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hamoid/video_export_processing/pull/51#issuecomment-386860826, or mute the thread https://github.com/notifications/unsubscribe-auth/AFE-gfhjGkiAcKdGZp0KMMkfGq3jV-Zyks5tvqoMgaJpZM4TtUZq .

-- Regards, Brett

[hamoid]

Are you logged in in GitHub? If you are and click Files Changed and scroll down to the middle of the page, I wrote this question: Are this (L285) and L299 swapped?

Also, maybe if you are logged in you may see a blue dot on the top bar in GitHub, meaning that you have notifications. If you click that you may see my question. It's also possible to receive notifications by e-mail (if wanted). I for instance subscribe to some projects so I receive by e-mail every commit, issue, pull request... everything by e-mail :) Just to follow what's going on.

[bcoley]

Yes, I'm logged in, but don't seem to see any notifications,

And when I go into the file I don't see anything in the source itself at https://github.com/hamoid/video_export_processing/blob/master/src/com/hamoid/VideoExport.java

I do see our email and comments at https://github.com/hamoid/video_export_processing/pull/51 ,

Files Changed looks like this for me:

and around the lines in question

Sorry to belabor this one, just wondering if I am doing something wrong or have a bad setting? Thanks.

On Mon, May 7, 2018 at 11:59 AM, Abe Pazos notifications@github.com wrote:

Are you logged in in GitHub? If you are and click Files Changed and scroll down to the middle of the page, I wrote this question: Are this (L285) and L299 swapped?

Also, maybe if you are logged in you may see a blue dot on the top bar in GitHub, meaning that you have notifications. If you click that you may see my question. It's also possible to receive notifications by e-mail (if wanted). I for instance subscribe to some projects so I receive by e-mail every commit, issue, pull request... everything by e-mail :) Just to follow what's going on.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hamoid/video_export_processing/pull/51#issuecomment-387112960, or mute the thread https://github.com/notifications/unsubscribe-auth/AFE-gYt_nBXv5WiDbf6u3OE6p1JqT2xaks5twG9ZgaJpZM4TtUZq .

-- Regards, Brett

[bcoley]

To get back to the question, those lines are what I intended. Basically, setAudioUrl() will complain if the caller had already supplied an audioFilePath before calling to set a url, and vice versa with setAudioFileName(). This is to have the callers choose a url or a file as the audio source, but not both.

To try to mix both of these together seemed to make no sense to me (url audio is gathered between startMovie() and endMovie(), a file is only processed after endMovie() ) nor did it appear to even be possible with ffmpeg (but then again, I realize I could be wrong on that ffmpeg assumption), just figured it would be better to steer users to one or the other.

In hindsight it occurs to me that the code in setAudioUrl() could check to see if the url was already set and complain, but it's not really going to hurt anything if they call that method multiple times, the value that matters is what was in effect at the time of startMovie(). Thanks!

On Mon, May 7, 2018 at 11:59 AM, Abe Pazos notifications@github.com wrote:

Are you logged in in GitHub? If you are and click Files Changed and scroll down to the middle of the page, I wrote this question: Are this (L285) and L299 swapped?

Also, maybe if you are logged in you may see a blue dot on the top bar in GitHub, meaning that you have notifications. If you click that you may see my question. It's also possible to receive notifications by e-mail (if wanted). I for instance subscribe to some projects so I receive by e-mail every commit, issue, pull request... everything by e-mail :) Just to follow what's going on.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hamoid/video_export_processing/pull/51#issuecomment-387112960, or mute the thread https://github.com/notifications/unsubscribe-auth/AFE-gYt_nBXv5WiDbf6u3OE6p1JqT2xaks5twG9ZgaJpZM4TtUZq .

-- Regards, Brett

[hamoid]

Thank you for the explanation! I just wanted to make sure :)

What do you think about having a boolean audioIsSet or something in that direction? I was just wondering what if there are 3 or 4 ways to set audio, those checks could become very complicated. Another option would be to have an int to set the audioMode, which by default is in UndefinedMode, but it could be set to UrlMode or FilePathMode (bad names, it's just about the concept). Then we could just check if it is in UndefinedMode, but maybe (I haven't thought about it) it could be useful to know in which mode it is. What do you think?

Btw, this is how that Files Changed page looks to me. I wonder if it's because I should add you as a developer or something... no idea.

[bcoley]

Having some booleans would be fine, but the code for now is ok. I'm not sure why I can't see that question (my screen pastes didn't show up on github, but I hoped you saw them in email). If you want, add me in and I'll work on it as time permits. Thanks!

On Mon, May 7, 2018 at 2:04 PM, Abe Pazos notifications@github.com wrote:

Thank you for the explanation! I just wanted to make sure :)

What do you think about having a boolean audioIsSet or something in that direction? I was just wondering what if there are 3 or 4 ways to set audio, those checks could become very complicated. Another option would be to have an int to set the audioMode, which by default is in UndefinedMode, but it could be set to UrlMode or FilePathMode (bad names, it's just about the concept). Then we could just check if it is in UndefinedMode, but maybe (I haven't thought about it) it could be useful to know in which mode it is. What do you think?

Btw, this is how that Files Changed page looks to me. I wonder if it's because I should add you as a developer or something... no idea.

[image: 2018-05-07-195612_1888x565_scrot] https://user-images.githubusercontent.com/108264/39716617-2a32ca3e-5231-11e8-9d62-53d3dfa5e881.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hamoid/video_export_processing/pull/51#issuecomment-387151408, or mute the thread https://github.com/notifications/unsubscribe-auth/AFE-gSgfws4JZdWAf-FfwqX3UmbS7TW-ks5twIyXgaJpZM4TtUZq .

-- Regards, Brett

[bcoley]

Does this pull request need to be merged before I start with anything else, like adding an enum or booleans to clean up some of the decision logic about what kind of audio is being used? Thanks.

[hamoid]

Sorry for the delay. I'll merge this later today so everything should be ok.