-
Notifications
You must be signed in to change notification settings - Fork 618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/issue 38 restart period #67
Changes from all commits
95ceceb
a2703f0
b611bd8
b0da64c
c230d54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,10 @@ public class StreamFetcherManager { | |
|
||
private ConcurrentLinkedQueue<StreamFetcher> streamFetcherList = new ConcurrentLinkedQueue<>(); | ||
|
||
private int streamCheckerInterval = 10000; | ||
/** | ||
* Time period in milli seconds for checking stream fetchers status, restart issues etc. | ||
*/ | ||
private int streamCheckerIntervalMs = 10000; | ||
|
||
private ISchedulingService schedulingService; | ||
|
||
|
@@ -45,21 +48,63 @@ public class StreamFetcherManager { | |
|
||
protected AtomicBoolean isJobRunning = new AtomicBoolean(false); | ||
|
||
public StreamFetcherManager(ISchedulingService schedulingService, IDataStore datastore,IScope scope) { | ||
public static class StreamFetcherFactory { | ||
public StreamFetcher make(Broadcast stream, IScope scope) throws Exception { | ||
return new StreamFetcher(stream, scope); | ||
} | ||
} | ||
|
||
private boolean restartStreamAutomatically = true; | ||
|
||
public StreamFetcherFactory streamFetcherFactory; | ||
|
||
/** | ||
* Time period in seconds for restarting stream fetchers | ||
*/ | ||
private int restartStreamFetcherPeriodSeconds; | ||
|
||
public StreamFetcherManager(ISchedulingService schedulingService, IDataStore datastore,IScope scope) { | ||
this(schedulingService, datastore, scope, null); | ||
} | ||
|
||
|
||
|
||
public StreamFetcherManager(ISchedulingService schedulingService, IDataStore datastore,IScope scope, StreamFetcherFactory streamFetcherFactory) { | ||
this.schedulingService = schedulingService; | ||
this.datastore = datastore; | ||
this.scope=scope; | ||
if (streamFetcherFactory != null) { | ||
this.streamFetcherFactory = streamFetcherFactory; | ||
} | ||
else { | ||
this.streamFetcherFactory = new StreamFetcherFactory(); | ||
} | ||
|
||
} | ||
|
||
public int getStreamCheckerInterval() { | ||
return streamCheckerInterval; | ||
return streamCheckerIntervalMs; | ||
} | ||
|
||
|
||
/** | ||
* Set stream checker interval, this value is used in periodically checking | ||
* the status of the stream fetchers | ||
* | ||
* @param streamCheckerInterval, time period of the stream fetcher check interval in milliseconds | ||
*/ | ||
public void setStreamCheckerInterval(int streamCheckerInterval) { | ||
this.streamCheckerInterval = streamCheckerInterval; | ||
this.streamCheckerIntervalMs = streamCheckerInterval; | ||
} | ||
|
||
/** | ||
* Set stream fetcher restart period, this value is used in periodically stopping and starting | ||
* stream fetchers. If this value is zero it will not restart stream fetchers | ||
* | ||
* @param restartStreamFetcherPeriod, time period of the stream fetcher restart period in seconds | ||
*/ | ||
public void setRestartStreamFetcherPeriod(int restartStreamFetcherPeriod) { | ||
this.restartStreamFetcherPeriodSeconds = restartStreamFetcherPeriod; | ||
} | ||
|
||
|
||
|
@@ -68,10 +113,12 @@ public Result startStreaming(Broadcast broadcast) { | |
Result result=new Result(false); | ||
|
||
try { | ||
StreamFetcher streamScheduler = new StreamFetcher(broadcast,scope); | ||
StreamFetcher streamScheduler = streamFetcherFactory.make(broadcast, scope); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of making StreamFetcherFactory a static class, make "make" method a static method, so there will be no need to create an instance and keep as variable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making make method static is ok. On the other hand, in testing we give a mock StreamFetcherFactory instance as a parameter to create a test case. How can we do that not using an instance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. StreamFetcherFactory class has only "make" method. You dont need an instance to call it if you make the "make" method static. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. Please just take a look at here We give a different mock StreamFactory class. If we call StreamFactory.make static function directly how we give a separate mock in the test case above? Maybe we need to change some other things in the code as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You will not give a mock and you will not pass as parameter. |
||
streamScheduler.setRestartStream(restartStreamAutomatically); | ||
streamScheduler.startStream(); | ||
|
||
if(broadcast.getType().equals(AntMediaApplicationAdapter.IP_CAMERA)) { | ||
String broadcastType = broadcast.getType(); | ||
if(broadcastType != null && broadcastType.equals(AntMediaApplicationAdapter.IP_CAMERA)) { | ||
try { | ||
Thread.sleep(6000); | ||
} catch (InterruptedException e) { | ||
|
@@ -85,6 +132,9 @@ public Result startStreaming(Broadcast broadcast) { | |
result.setSuccess(true); | ||
} | ||
streamFetcherList.add(streamScheduler); | ||
if (streamFetcherScheduleJobName == null) { | ||
scheduleStreamFetcherJob(); | ||
} | ||
} | ||
catch (Exception e) { | ||
e.printStackTrace(); | ||
|
@@ -113,11 +163,17 @@ public void startStreams(List<Broadcast> streams) { | |
startStreaming(streams.get(i)); | ||
} | ||
|
||
scheduleStreamFetcherJob(); | ||
} | ||
|
||
private void scheduleStreamFetcherJob() { | ||
if (streamFetcherScheduleJobName != null) { | ||
schedulingService.removeScheduledJob(streamFetcherScheduleJobName); | ||
} | ||
|
||
streamFetcherScheduleJobName = schedulingService.addScheduledJobAfterDelay(streamCheckerInterval, new IScheduledJob() { | ||
streamFetcherScheduleJobName = schedulingService.addScheduledJobAfterDelay(streamCheckerIntervalMs, new IScheduledJob() { | ||
|
||
private int lastRestartCount = 0; | ||
|
||
@Override | ||
public void execute(ISchedulingService service) throws CloneNotSupportedException { | ||
|
@@ -127,10 +183,19 @@ public void execute(ISchedulingService service) throws CloneNotSupportedExceptio | |
streamCheckerCount++; | ||
|
||
logger.warn("StreamFetcher Check Count :" + streamCheckerCount); | ||
|
||
int countToRestart = 0; | ||
if (restartStreamFetcherPeriodSeconds > 0) | ||
{ | ||
int streamCheckIntervalSec = streamCheckerIntervalMs / 1000; | ||
countToRestart = (streamCheckerCount * streamCheckIntervalSec) / restartStreamFetcherPeriodSeconds; | ||
} | ||
|
||
|
||
if (streamCheckerCount % 180 == 0) { | ||
if (countToRestart > lastRestartCount) { | ||
|
||
logger.info("Restarting streams"); | ||
lastRestartCount = countToRestart; | ||
logger.info("This is {} times that restarting streams", lastRestartCount); | ||
for (StreamFetcher streamScheduler : streamFetcherList) { | ||
|
||
if (streamScheduler.isStreamAlive()) | ||
|
@@ -156,19 +221,11 @@ public void execute(ISchedulingService service) throws CloneNotSupportedExceptio | |
AntMediaApplicationAdapter.BROADCAST_STATUS_FINISHED); | ||
} | ||
} | ||
/* | ||
if (!streamScheduler.isThreadActive()) { | ||
streamScheduler.startStream(); | ||
} | ||
else { | ||
logger.info("there is an active thread for {} so that new thread is not started", stream.getStreamId()); | ||
} | ||
*/ | ||
} | ||
} | ||
} | ||
} | ||
}, 5000); | ||
}, streamCheckerIntervalMs); | ||
|
||
logger.info("StreamFetcherSchedule job name {}", streamFetcherScheduleJobName); | ||
} | ||
|
@@ -189,4 +246,16 @@ public void setStreamFetcherList(ConcurrentLinkedQueue<StreamFetcher> streamFetc | |
this.streamFetcherList = streamFetcherList; | ||
} | ||
|
||
|
||
|
||
public boolean isRestartStreamAutomatically() { | ||
return restartStreamAutomatically; | ||
} | ||
|
||
|
||
|
||
public void setRestartStreamAutomatically(boolean restartStreamAutomatically) { | ||
this.restartStreamAutomatically = restartStreamAutomatically; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,11 @@ | |
import org.junit.Before; | ||
import org.junit.BeforeClass; | ||
import org.junit.Test; | ||
import org.mockito.Mockito; | ||
|
||
import static org.mockito.Mockito.*; | ||
import org.mongodb.morphia.Datastore; | ||
import org.red5.server.scheduling.QuartzSchedulingService; | ||
import org.red5.server.scope.WebScope; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
@@ -43,7 +47,10 @@ | |
import io.antmedia.integration.MuxingTest; | ||
import io.antmedia.integration.RestServiceTest; | ||
import io.antmedia.muxer.MuxAdaptor; | ||
import io.antmedia.rest.model.Result; | ||
import io.antmedia.streamsource.StreamFetcher; | ||
import io.antmedia.streamsource.StreamFetcherManager; | ||
import io.antmedia.streamsource.StreamFetcherManager.StreamFetcherFactory; | ||
|
||
@ContextConfiguration(locations = { "test.xml" }) | ||
public class StreamFetcherUnitTest extends AbstractJUnit4SpringContextTests { | ||
|
@@ -104,7 +111,6 @@ public void before() { | |
//reset values in the bean | ||
getAppSettings().setMp4MuxingEnabled(defaultSettings.isMp4MuxingEnabled()); | ||
getAppSettings().setHlsMuxingEnabled(defaultSettings.isHlsMuxingEnabled()); | ||
getAppSettings().setAddDateTimeToMp4FileName(false); | ||
|
||
getAppSettings().setMp4MuxingEnabled(true); | ||
getAppSettings().setAddDateTimeToMp4FileName(defaultSettings.isAddDateTimeToMp4FileName()); | ||
|
@@ -148,7 +154,10 @@ public void testBugUpdateStreamFetcherStatus() { | |
//set mapdb datastore to stream fetcher because in memory datastore just have references and updating broadcst | ||
// object updates the reference in inmemorydatastore | ||
app.getStreamFetcherManager().setDatastore(dataStore); | ||
|
||
|
||
app.getStreamFetcherManager().setRestartStreamAutomatically(false); | ||
app.getStreamFetcherManager().setStreamCheckerInterval(5000); | ||
|
||
app.getStreamFetcherManager().getStreamFetcherList().clear(); | ||
|
||
assertEquals(0, app.getStreamFetcherManager().getStreamFetcherList().size()); | ||
|
@@ -216,6 +225,85 @@ public void testBugUpdateStreamFetcherStatus() { | |
logger.info("leaving testBugUpdateStreamFetcherStatus"); | ||
|
||
} | ||
|
||
|
||
@Test | ||
public void testRestartPeriodStreamFetcher() { | ||
|
||
try { | ||
//Create Stream Fetcher Manager | ||
QuartzSchedulingService scheduler = (QuartzSchedulingService) applicationContext.getBean(QuartzSchedulingService.BEAN_NAME); | ||
|
||
InMemoryDataStore memoryDataStore = new InMemoryDataStore("testdb"); | ||
|
||
|
||
|
||
//Create a mock StreamFetcher and add it to StreamFetcherManager | ||
StreamFetcher streamFetcher = Mockito.mock(StreamFetcher.class); | ||
Broadcast stream = Mockito.mock(Broadcast.class); | ||
|
||
stream.setStreamId(String.valueOf((Math.random() * 100000))); | ||
|
||
stream.setStreamUrl("anyurl"); | ||
streamFetcher.setStream(stream); | ||
when(streamFetcher.getStream()).thenReturn(stream); | ||
when(streamFetcher.isStreamAlive()).thenReturn(true); | ||
when(streamFetcher.getCameraError()).thenReturn(new Result(true)); | ||
|
||
StreamFetcherFactory factory = mock(StreamFetcherFactory.class); | ||
when(factory.make(stream, appScope)).thenReturn(streamFetcher); | ||
|
||
StreamFetcherManager fetcherManager = new StreamFetcherManager(scheduler, memoryDataStore, appScope, factory); | ||
|
||
//set checker interval to 3 seconds | ||
fetcherManager.setStreamCheckerInterval(4000); | ||
|
||
//set restart period to 5 seconds | ||
fetcherManager.setRestartStreamFetcherPeriod(5); | ||
|
||
//Start stream fetcher | ||
Result result = fetcherManager.startStreaming(stream); | ||
assertTrue(result.isSuccess()); | ||
|
||
//wait 10-12 seconds | ||
Thread.sleep(13000); | ||
|
||
//check that stream fetcher stop and start stream is called 4 times | ||
verify(streamFetcher, times(2)).stopStream(); | ||
|
||
//it is +1 because it is called at first start | ||
verify(streamFetcher, times(3)).startStream(); | ||
verify(streamFetcher, times(3)).startStream(); | ||
|
||
//set restart period to 0 seconds | ||
fetcherManager.setRestartStreamFetcherPeriod(0); | ||
|
||
//wait 10-12 seconds | ||
Thread.sleep(13000); | ||
|
||
//check that stream fetcher stop and start stream is not called | ||
verify(streamFetcher, times(2)).stopStream(); | ||
verify(streamFetcher, times(3)).startStream(); | ||
|
||
//set restart period to 0 seconds | ||
fetcherManager.setRestartStreamFetcherPeriod(5); | ||
|
||
//wait 10-12 seconds | ||
Thread.sleep(13000); | ||
|
||
//check that stream fetcher stop and start stream is not called | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be a separate test for each verifying a feature. Ex: each verify after Thread.sleeps look like testing a different thing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually this is a kind of scenario testing. There is periodic task in StreamFetcher, we change some metrics on the fly and verify periodic task is working properly. Change again some metrics and verify test case again. This is real use case scenario. We need to sleep sometime to make sure the periodic task do its job. |
||
verify(streamFetcher, atLeast(4)).stopStream(); | ||
verify(streamFetcher, atLeast(5)).startStream(); | ||
|
||
fetcherManager.setRestartStreamFetcherPeriod(0); | ||
|
||
} catch (Exception e) { | ||
e.printStackTrace(); | ||
fail(e.getMessage()); | ||
} | ||
|
||
|
||
} | ||
|
||
@Test | ||
public void testThreadStopStart() { | ||
|
@@ -698,9 +786,8 @@ public void testHLSFlagResult() { | |
try { | ||
String textInFile; | ||
|
||
startCameraEmulator(); | ||
|
||
Broadcast newCam = new Broadcast("streamSource", "127.0.0.1:8080", "admin", "admin", "rtsp://127.0.0.1:6554/test.flv", | ||
Broadcast newCam = new Broadcast("streamSource", "127.0.0.1:8080", "admin", "admin", "src/test/resources/test.ts", | ||
AntMediaApplicationAdapter.STREAM_SOURCE); | ||
|
||
assertNotNull(newCam.getStreamUrl()); | ||
|
@@ -710,6 +797,9 @@ public void testHLSFlagResult() { | |
assertNotNull(newCam.getStreamId()); | ||
|
||
StreamFetcher fetcher = new StreamFetcher(newCam, appScope); | ||
|
||
|
||
fetcher.setRestartStream(false); | ||
|
||
assertFalse(fetcher.isThreadActive()); | ||
assertFalse(fetcher.isStreamAlive()); | ||
|
@@ -758,8 +848,6 @@ public void testHLSFlagResult() { | |
|
||
assertFalse(textInFile.contains("EXT-X-ENDLIST")); | ||
|
||
stopCameraEmulator(); | ||
|
||
getInstance().getDataStore().delete(id); | ||
} | ||
catch (Exception e) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to make it more readable, this might be an option:
this.streamFetcherFactory = streamFetcherFactory;
if(this.streamFetcherFactory == null){
this.streamFetcherFactory = new StreamFetcherFactory();}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check this out
e3f6052
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mekya add this also this.streamFetcherFactory = streamFetcherFactory;