[haiku-commits] Re: haiku: hrev46004 - src/kits/media headers/os/media src/apps

  • From: John Scipione <jscipione@xxxxxxxxx>
  • To: "haiku-commits@xxxxxxxxxxxxx" <haiku-commits@xxxxxxxxxxxxx>
  • Date: Sat, 31 Aug 2013 16:58:25 -0400

On Sat, Aug 31, 2013 at 4:16 PM, Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:
> 1. Library code should not print anything, unless it is supposed to.

Okay, I'll hide the printf's behind a #ifdef TRACE_MEDIA_TRACK guards.
I was fooled because the library printf's elsewhere. My bad.

> 2. Calling debugger() is just fine to complain about programming errors.

You really think it's okay to drop the user into the debugger when
they try to play a bad video file? That's pretty harsh, the program
can handle a bad file.

> In either case I suppose an error code should be returned after detecting
> and complaining about the problem (or maybe the code handles the issue later
> somehow?). debugger() does not crash the program. It just drops it into the
> debugger. One can continue the program.

The error code has already been set by the add-on, MediaPlayer and
MediaConverter both detect the error and alert.

I see, that's a pretty small distinction, but now I understand better,
thank you.

On Sat, Aug 31, 2013 at 4:16 PM, Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:
> On 08/31/2013 09:58 PM, jscipione@xxxxxxxxx wrote:
>>
>>
>> ############################################################################
>>
>> Commit:      72d0921ae8a63926e0e49667a6c22bef64691227
>> URL:         http://cgit.haiku-os.org/haiku/commit/?id=72d0921
>> Author:      John Scipione <jscipione@xxxxxxxxx>
>> Date:        Sat Aug 31 19:54:21 2013 UTC
>>
>> MediaTrack: Calling debugger() crashes the program
>>
>> So, call printf instead, which allows the program to print an error
>> rather than crashing.
>>
>>
>> ----------------------------------------------------------------------------
>>
>> diff --git a/src/kits/media/MediaTrack.cpp b/src/kits/media/MediaTrack.cpp
>> index 47173d8..c9e4df6 100644
>> --- a/src/kits/media/MediaTrack.cpp
>> +++ b/src/kits/media/MediaTrack.cpp
>> @@ -20,8 +20,6 @@
>>
>>   #include <Roster.h>
>>
>> -#include "debug.h"
>> -
>>   #include "MediaExtractor.h"
>>   #include "MediaWriter.h"
>>   #include "PluginManager.h"
>> @@ -217,26 +215,43 @@ BMediaTrack::DecodedFormat(media_format* _format,
>> uint32 flags)
>>         printf("BMediaTrack::DecodedFormat: nego: %s\n", s);
>>   #endif
>>
>> -       if (_format->type == 0)
>> -               debugger("Decoder didn't set output format type");
>> +       if (_format->type == 0) {
>> +               printf("BMediaTrack::DecodedFormat: Decoder didn't set
>> output format "
>> +                       "type");
>> +       }
>>
>>         if (_format->type == B_MEDIA_RAW_AUDIO) {
>> -               if (_format->u.raw_audio.byte_order == 0)
>> -                       debugger("Decoder didn't set raw audio output byte
>> order");
>> -               if (_format->u.raw_audio.format == 0)
>> -                       debugger("Decoder didn't set raw audio output
>> sample format");
>> -               if (_format->u.raw_audio.buffer_size <= 0)
>> -                       debugger("Decoder didn't set raw audio output
>> buffer size");
>> +               if (_format->u.raw_audio.byte_order == 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw audio "
>> +                               "output byte order.\n");
>> +               }
>> +               if (_format->u.raw_audio.format == 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw audio "
>> +                               "output sample format.\n");
>> +               }
>> +               if (_format->u.raw_audio.buffer_size <= 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw audio "
>> +                               "output buffer size.\n");
>> +               }
>>         }
>> +
>>         if (_format->type == B_MEDIA_RAW_VIDEO) {
>> -               if (_format->u.raw_video.display.format == 0)
>> -                       debugger("Decoder didn't set raw video output
>> color space");
>> -               if (_format->u.raw_video.display.line_width == 0)
>> -                       debugger("Decoder didn't set raw video output
>> line_width");
>> -               if (_format->u.raw_video.display.line_count == 0)
>> -                       debugger("Decoder didn't set raw video output
>> line_count");
>> -               if (_format->u.raw_video.display.bytes_per_row == 0)
>> -                       debugger("Decoder didn't set raw video output
>> bytes_per_row");
>> +               if (_format->u.raw_video.display.format == 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw video "
>> +                               "output color space.\n");
>> +               }
>> +               if (_format->u.raw_video.display.line_width == 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw video "
>> +                               "output line_width.\n");
>> +               }
>> +               if (_format->u.raw_video.display.line_count == 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw video "
>> +                               "output line_count.\n");
>> +               }
>> +               if (_format->u.raw_video.display.bytes_per_row == 0) {
>> +                       printf("BMediaTrack::DecodedFormat: Decoder didn't
>> set raw video "
>> +                               "output bytes_per_row.\n");
>> +               }
>>         }
>>
>>         if ((flags & B_MEDIA_DISABLE_FORMAT_TRANSLATION) == 0) {
>
>
> 1. Library code should not print anything, unless it is supposed to.
> 2. Calling debugger() is just fine to complain about programming errors.
>
> In either case I suppose an error code should be returned after detecting
> and complaining about the problem (or maybe the code handles the issue later
> somehow?). debugger() does not crash the program. It just drops it into the
> debugger. One can continue the program.
>
> CU, Ingo
>
>

Other related posts: