[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:
