[Live-devel] Problem with fix in MPEG2TransportStreamFromESSource stops frame delivery

[Bruno Abreu]

On 01/22/2013 06:32 PM, Ross Finlayson wrote:
>> We think this change was introduced in the sequence of:
>> 2012.09.11:
>> - Fixed a bug in "MPEG2TransportStreamFromESSource": Its destructor wasn't
>> stopping the delivery from upstream objects.
>
> That's correct.  That line
> doStopGettingFrames();
> was added to the "MPEG2TransportStreamFromESSource" destructor for a
> good reason.  You should NOT be deleting that line.
>
> If you have a "MPEG2TransportStreamFromESSource" as a replica, then when
> the  "MPEG2TransportStreamFromESSource" destructor gets called - for
> whatever reason - then that will cause
> "StreamReplica::doStopGettingFrames()" to be called first, then
> "StreamReplica::~StreamReplica()".  Neither of which should be causing
> any problems for you.

Of course. I didn't mean that fix was wrong, just that it exposed a problem that
must lie elsewhere and, for us, the immediate solution to keep things running
was to remove that line.

And, in fact, we just found out where the real problem is. It's in StreamReplicator.

In order to create the stream's SDP description, the createNewStreamSource()
method from an OnDemandServerMediaSubsession is called twice. The first returned
FramedSource is immediately destructed and then a new one is created. We've been
observing this behavior for a long time, and it's fine.

In our case this means that a StreamReplica (of our live DeviceSource) will be
created and immediately destructed, without (and this is what causes the
problem) ever having been activated.

Since our replica feeds a MPEG2TransportStreamFromESSource, the introduction of
a call to doStopGettingFrames() on MPEG2TransportStreamFromESSource's destructor
causes StreamReplica::doStopGettingFrames() to be called also. And the problem
is that the replica wasn't activated (getNextFrame() was never called) but
deactivateStreamReplica() will be called (line 318) and will decrease the
fNumActiveReplicas counter. From then on we see the "Internal Error 2" message
(which we hadn't noticed before, sorry!) and everything stops.

The following patch did solve this problem and, this time, I think this is a
correct solution:

@@ -316,4 +316,6 @@
 void StreamReplica::doStopGettingFrames() {
-  fFrameIndex = -1; // When we start reading again, this will tell the
replicator that we were previously inactive.
-  fOurReplicator.deactivateStreamReplica(this);
+  if (fFrameIndex != -1) { // this might not have been activated at all
+    fFrameIndex = -1; // When we start reading again, this will tell the
replicator that we were previously inactive.
+    fOurReplicator.deactivateStreamReplica(this);
+  }
 }

Can this be fixed in next release?

Thank you,
Bruno Abreu 


PS - I'm having mail server problems so this is a new message, not a reply to
the previous one on this topic. Please accept my apologies if it creates a new
thread.


-- 
Living Data - Sistemas de Informação e Apoio à Decisão, Lda.

LxFactory - Rua Rodrigues de Faria, 103,
edifício I - 4º piso                  Phone:  +351 213622163
1300-501 LISBOA                       Fax:    +351 213622165
Portugal                              URL: www.livingdata.pt