owaisawan / xuggle

Automatically exported from code.google.com/p/xuggle
0 stars 0 forks source link

red5 transcoder should accept on-disk files #208

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
com.xuggler.red5.Transcoder currently only does red5<->red5.  Being able to 
convert file<->red5 
would be very useful.  I had to do it for a project of mine, so I figured I'd 
submit it back.  :)

The attached patch file refactors Transcoder to take ISimpleMediaFile's as 
input and output.  The 
ISimpleMediaFile's could be on-disk files or they can represent red5 streams.  
For red5 streams, 
they're created from the new factories 
Red5StreamingInputHandler#makeRed5Handler and 
Red5StreamingOutputHandler#makeRed5Handler.  See 
AudioTranscoderDemo/VideoTranscoderDemo.

fyi, the patch was made from r915 of trunk/java/xuggle-xuggler-red5.

Original issue reported on code.google.com by paleozogt on 9 Sep 2009 at 11:02

Attachments:

GoogleCodeExporter commented 9 years ago
fyi, there are a few issues with this patch:

1) If you try to convert raw pcm <-> a red5 stream, xuggler will crash and take 
out red5 with it (see issue #203).  
I'm currently using xuggler v2 for my own project as a work-around.

2) Stopping playback isn't done quite right in 
com.xuggler.red5.io.BroadcastStream#stop.  It used to be a no-op, but 
now it sends an NS_PLAY_STOP to the client.  The problem is that this is 
dissociated from when the stream actually 
finishes playing and can arrive too early.  Any guidance on how to proceed 
there is appreciated.

Original comment by paleozogt on 9 Sep 2009 at 11:03

GoogleCodeExporter commented 9 years ago
Hi there,

Are you willing to donate this patch under LGPL to Xuggler (we require LGPL for 
all
patches, even though the public free license is AGPL)?

- Art

Original comment by art.cla...@gmail.com on 9 Sep 2009 at 11:28

GoogleCodeExporter commented 9 years ago
Yes.

Original comment by paleozogt on 10 Sep 2009 at 1:10

GoogleCodeExporter commented 9 years ago
Hi Aaron (I think it's Aaron),

Can you regenerate this patch against the current tip of tree?  I think it's 
from an
older version?

- Art

Original comment by art.cla...@gmail.com on 11 Sep 2009 at 3:42

GoogleCodeExporter commented 9 years ago
I regenerated the patch against r922 of 
http://xuggle.googlecode.com/svn/trunk/java/xuggle-
xuggler-red5 (see attached).  That didn't seem to change the patch much (other 
than the 
revision number).  

fyi, I'm generating the patch like this:
   svn diff --diff-cmd=diff > xuggle-xuggler-red5_r922.patch

Perhaps I'm doing something wrong?

Original comment by paleozogt on 11 Sep 2009 at 3:58

Attachments:

GoogleCodeExporter commented 9 years ago
Oh, yeah, and it is Aaron. ;)

Original comment by paleozogt on 11 Sep 2009 at 3:59

GoogleCodeExporter commented 9 years ago
No, that's got it.  Thanks!

FYI I'm looking at this today but still no ETA on acceptance.  I'm 
concentrating on
the PCM crash.

A quick review of this patch seemed to suggest it might bring in a new Apache
dependency... is that correct?  If so, does Red5 already depend on it?

Original comment by art.cla...@gmail.com on 11 Sep 2009 at 4:05

GoogleCodeExporter commented 9 years ago
Are you referring to these lines in Transcoder#transcode:
    if (log.isDebugEnabled())
      log.debug("input : {}", ReflectionToStringBuilder.toString(mInputInfo, ToStringStyle.MULTI_LINE_STYLE));
    if (log.isDebugEnabled())
      log.debug("output: {}", ReflectionToStringBuilder.toString(mOutputInfo, ToStringStyle.MULTI_LINE_STYLE));

ReflectionToStringBuilder is part of commons-lang-2.4.jar, which is included in 
the lib folder of the red5 distro in v0.8 
and v0.9.

Original comment by paleozogt on 11 Sep 2009 at 4:15

GoogleCodeExporter commented 9 years ago
That's the pieces.  In general I prefer not to add a dependency -- even if Red5
already has it.  Since it's just logging, I think we can probably replace with a
standard toString() once it's working?

Original comment by art.cla...@gmail.com on 11 Sep 2009 at 4:31

GoogleCodeExporter commented 9 years ago
That's fine-- its just logging after all.  You can take it out.  Though I find 
that having that info 
put into the log greatly eases debugging.

btw, toString won't work because SimpleMediaFile doesn't implement it.  Using 
ReflectionToStringBuilder was the fastest way for me to get logging for all of 
ISimpleMediaFile's 
properties (and I wanted to limit the patch to just the red5 part of xuggler).

I can submit a patch for SimpleMediaFile that will implement a toString, though 
I'm inclined to use ReflectionToStringBuilder (or some kind of reflection tool 
to walk the properties) as its easier to 
maintain.

Original comment by paleozogt on 11 Sep 2009 at 4:41

GoogleCodeExporter commented 9 years ago
Since issue #203 has been fixed, the first issue with this patch mentioned 
above has 
also been fixed.

Original comment by paleozogt on 14 Sep 2009 at 9:51

GoogleCodeExporter commented 9 years ago
Here's another patch (against r927).  It has a fix for the 'dangling container'
problem (as seen on the mailing list).  It also has an attempt at getting the
stop-playback timing fixed by throttling the output packet rate (based on your
suggestion, art).  The throttling approach doesn't work very well, but its 
better
than before.

Original comment by paleozogt on 24 Sep 2009 at 4:29

Attachments:

GoogleCodeExporter commented 9 years ago
My approach for throttling the output packets in the previous patch was dumb. 
Attached is a new patch with a not-dumb way of doing it that works much better. 
:)  

(fyi, it still includes the 'dangling container' fix.)

Original comment by paleozogt on 24 Sep 2009 at 6:07

Attachments:

GoogleCodeExporter commented 9 years ago
Hi,

Is it possible with this patch to stream media files supported by ffmpeg, lets 
say a divx/mp3 in an avi container, 
directly from the streams of a webapp?

Sincerely,
hbwinther.

Original comment by hbwint...@gmail.com on 18 Oct 2009 at 8:50

GoogleCodeExporter commented 9 years ago
I think so, yes.

Original comment by paleozogt on 19 Oct 2009 at 3:09

GoogleCodeExporter commented 9 years ago

Original comment by art.cla...@gmail.com on 5 Jan 2010 at 9:17

GoogleCodeExporter commented 9 years ago
Hi Aaron,

We've decided to deprecate our original approach to the Red5 adapter, so we're
closing this out.  The new recommended approach will be 100% decoupled from Red5
(i.e. you'll chat via RTMP streams).

We may create a new Red5 interface method in the future, but we want Red5 to 
reach
1.0 and be stable for a while before we attempt that.

Original comment by art.cla...@gmail.com on 16 Jan 2010 at 3:03

GoogleCodeExporter commented 9 years ago
Is the new red5 adapter approach in svn yet?  I'd like to try it out.

Original comment by paleozogt on 16 Jan 2010 at 3:46

GoogleCodeExporter commented 9 years ago
The alpha is; we're just recommending people talk RTMP directly over localhost. 
 Use
the "rtmp://localhost/yourapp/yourstream" to play or publish.

This is broken with Red5 tip of tree right now, but works with versions older 
than
last week.  It works with Wowza and FMS.

Original comment by art.cla...@gmail.com on 16 Jan 2010 at 5:46