Adding support for audio from a streaming internet radio station, example program included.
Adding support for audio from a streaming internet radio station, example program included.
Add audio url support #51
Merged
bcoley
merged 3 commits from 2018年05月21日 22:20:48 +02:00
add-audio-url-support into master
hamoid
(Migrated from github.com)
reviewed 2018年05月01日 07:15:14 +02:00
hamoid
(Migrated from github.com)
left a comment
Looks great!
Looks great!
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!
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!
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.
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.
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
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
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
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
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.
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.
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
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
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
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
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.
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.
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
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
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.
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.
Sorry for the delay. I'll merge this later today so everything should be ok.
Sorry for the delay. I'll merge this later today so everything should be ok.
hamoid
(Migrated from github.com)
approved these changes 2018年05月21日 22:20:26 +02:00
hamoid
(Migrated from github.com)
commented 2018年05月01日 07:15:59 +02:00
Are this and L299 swapped?
Are this and L299 swapped?
Sign in to join this conversation.
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set.
Reference
hamoid/video_export_processing!51
Loading...
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "add-audio-url-support"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?