<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">

<html xmlns="http://www.w3.org/1999/xhtml"><head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
 </head><body><p><br></p><blockquote type="cite"><p>On 07 April 2016 at 02:22 MythTV <noreply@mythtv.org> wrote:<br><br><br>#12709: Segmentation fault playing a recording with file missing<br>-----------------------------------------+-----------------------------<br> Reporter: Peter Bennett <pgbennett@…> | Owner: jyavenard<br> Type: Bug Report - Crash | Status: new<br> Priority: minor | Milestone: 0.28<br>Component: MythTV - Video Playback | Version: Master Head<br> Severity: medium | Resolution:<br> Keywords: | Ticket locked: 0<br>-----------------------------------------+-----------------------------<br><br>Comment (by knight):<br><br> Hi Roger!<br><br> Replying to [comment:2 rsiddons]:<br> > The video player (mythfrontend::main::internal_play_media()) plays video<br> from any source. For instance Gallery uses it for camera videos and<br> !MythBrowser and !SetupWizard use it for downloaded videos. So it has to<br> fail in a generic/independent way.<br> ><br> > "Try it and handle failure" is a more robust strategy than "Check first,<br> do it and handle unexpected failure" (file disappears between check and<br> play ?). It also avoids having to retrieve a remote file twice.<br><br> Assuming I correctly understood what you said I do not fully agree with<br> this...<br><br> The null pointer check should be there regardless but it sounds to me like<br> it is a null pointer because the file is missing so we already know before<br> we reach that point that something is wrong.<br><br> Why should we rely on a side effect of protecting our code against coding<br> errors and the like to handle something which should be handle long before<br> starting to initialize the UI.<br><br> I have not tried Peter's patch but unless you tell me that with it in<br> place the user is told what is actually wrong I do not consider that only<br> handling the null pointer at this point is a good solution. From a<br> programmer standpoint we "handled" the problem and did not crash but it<br> would not be very user friendly.<br><br> If we want it to fail in a generic/independent way maybe we should<br> actually remove all the code that does file handling from all of this<br> function/method client's and (Gallery, MythBrowser, etc...), make it<br> common and make it handle all of the possible failures this should<br> encounter.<br><br> Yes, we should protect our code against using null pointers but this<br> should not be the only thing we do to handle those kind of errors...<br><br> I am very far from a C++ guru but in the languages I do program in I both<br> program my code against using null pointers (even if it supposed to have<br> been taken care of earlier) and try to catch the error as soon as we know<br> it exists...<br><br> We are way too close to release to do little more than what Peter put in<br> place however so this solution so Peter's current forthcoming solution<br> will have to do...</p></blockquote><p>The offending commit is only in Master. It doesn't affect RC-1 or fixes/0.28.<br></p><p>The NULL indicates that the player didn't start for some undefined reason. A User Notification is raised separately with the specific reason.<br></p><p>However, looking closer, your/Peter's original question is good. HandleStateChange initialises the UI at the end even though it already knows the player has failed.<br></p><p><a href="https://code.mythtv.org/trac/browser/mythtv/mythtv/libs/libmythtv/tv_play.cpp?rev=5ca0ddeab014daaaa2d7e8eeb5bbf6b095202a86#L2595">https://code.mythtv.org/trac/browser/mythtv/mythtv/libs/libmythtv/tv_play.cpp?rev=5ca0ddeab014daaaa2d7e8eeb5bbf6b095202a86#L2595</a></p><p>As a quick test, I ignored this block when "ok == false" and it worked fine: it failed a lot quicker without all the flashing. So IMO that's something worth pursuing. Maybe someone who's familiar with this code will comment ?<br></p></body></html>