cecd-devel Mailing List for CECD
Status: Beta
Brought to you by:
pbatard
Menu
▾
▴
cecd-devel — libcec and cecd development
You can subscribe to this list here.
| 2010 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
(3) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2011 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
(1) |
Jul
|
Aug
(16) |
Sep
(4) |
Oct
|
Nov
|
Dec
(29) |
| 2012 |
Jan
(15) |
Feb
|
Mar
(1) |
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Florian F. <f.f...@gm...> - 2012-03-02 11:08:15
|
Hello all, While playing with cecd/libcec, I noticed that some TVs have a behavior which is out of specifications, in particular with respect to the following messages: - Set OSD String: none of the TVs that I have (a Toshiba and a Sony Bravia) do not show anything on their screen - Volume changes: since the TVs do not implement the Audio system capability, we must change the remote volume using the CEC_UI* key codes, I get the following behavior: * the Sony TVs ACK the message but does not do anything nor does it display a volume bar * the Toshiba TV replies with with an Abort opcode and a Refused reason, but it does show the volume bar and actually change the volume (software bug?) Is this something you also notice with your CEC devices as well? Thank you. -- Florian |
|
From: Pete B. <pb...@gm...> - 2012-01-18 10:26:42
|
On 2012.01.18 08:49, Florian Fainelli wrote: > Pushed to master. Great. Thanks for this. /Pete |
|
From: Florian F. <f.f...@gm...> - 2012-01-18 08:49:58
|
Hello,
On 01/17/12 18:06, Florian Fainelli wrote:
> ---
> libcec/decoder.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/libcec/decoder.h b/libcec/decoder.h
> index 1f4ee25..4ec2f61 100644
> --- a/libcec/decoder.h
> +++ b/libcec/decoder.h
> @@ -519,7 +519,7 @@ typedef struct {
> #define CEC_UI_NUMBER_9 0x29
> #define CEC_UI_DOT 0x2A
> #define CEC_UI_ENTER 0x2B
> -#define CEC_UI_CLEAR 0x2B
> +#define CEC_UI_CLEAR 0x2C
> #define CEC_UI_NEXT_FAVORITE 0x2F
> #define CEC_UI_CHANNEL_UP 0x30
> #define CEC_UI_CHANNEL_DOWN 0x31
Pushed to master.
--
Florian
|
|
From: Florian F. <f.f...@gm...> - 2012-01-17 17:06:44
|
---
libcec/decoder.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libcec/decoder.h b/libcec/decoder.h
index 1f4ee25..4ec2f61 100644
--- a/libcec/decoder.h
+++ b/libcec/decoder.h
@@ -519,7 +519,7 @@ typedef struct {
#define CEC_UI_NUMBER_9 0x29
#define CEC_UI_DOT 0x2A
#define CEC_UI_ENTER 0x2B
-#define CEC_UI_CLEAR 0x2B
+#define CEC_UI_CLEAR 0x2C
#define CEC_UI_NEXT_FAVORITE 0x2F
#define CEC_UI_CHANNEL_UP 0x30
#define CEC_UI_CHANNEL_DOWN 0x31
--
1.7.5.4
|
|
From: Florian F. <f.f...@gm...> - 2012-01-06 14:11:38
|
Hello Pete,
On 01/06/12 01:38, Pete Batard wrote:
> On 2012.01.05 17:13, Florian Fainelli wrote:
>> The current logical address allocation will iterate over all possible
>> logical addresses no matter what device type we configured. According
>> to the specification this is wrong, for instance, if we are a recording
>> device, we should:
>>
>> - take the unregistered address
>> - poll the<N>th logical address corresponding to a recording device
>> * if available, take it
>> * if not available continue to the next logical address
>> - until we either have a valid address or remain unregistered
>
> I think this is what we are already doing.
>
> Internally, we will iterate over all addresses, but it is not because we
> iterate them that we poll.
>
> To illustrate this, let's take the case of a playback device
> (device_type 4). The beginning of the loop code:
>
> for (logical_address = 1; logical_address< 15; logical_address++) {
> if (logical_address_table[logical_address] != device_type) {
> continue;
> }
>
> along with the fact that logical_address_table is defined as:
>
> logical_address_table[15] = {0, 1, 1, 3, 4, 5, 3, 3, 4, 1, 3, 4, 2, 2, 0};
>
> means that for logical_addresses 1 to 3, we don't do anything - we just
> increment logical_address and continue because the device_type won't
> match. Then, for logical_address 4, we have a match on the device_type,
> so we poll to see if the address is in use. If it is, then we will skip
> over logical_address 5, because the device_type won't match either and
> it's only for logical_address 8 that we will poll again.
You are right, I completely missed the use of the logical_address_table
and the use of the continue in the for loop. cecd seems to be doing it
right already.
>
> Thus, I believe that the use of a simple table and the insertion of
> continue statements achieves exactly what we need, and is a lot simpler
> than the solution your proposed.
>
>
> On the other hand, I have applied the polling message fix (good call)
> and also fixed the file permissions that got changed with the version bump.
>
> Regards,
>
> /Pete
>
> ------------------------------------------------------------------------------
> Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
> infrastructure or vast IT resources to deliver seamless, secure access to
> virtual desktops. With this all-in-one solution, easily deploy virtual
> desktops for less than the cost of PCs and save 60% on VDI infrastructure
> costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox
> _______________________________________________
> cecd-devel mailing list
> cec...@li...
> https://lists.sourceforge.net/lists/listinfo/cecd-devel
|
|
From: Pete B. <pb...@gm...> - 2012-01-06 00:38:09
|
On 2012.01.05 17:13, Florian Fainelli wrote:
> The current logical address allocation will iterate over all possible
> logical addresses no matter what device type we configured. According
> to the specification this is wrong, for instance, if we are a recording
> device, we should:
>
> - take the unregistered address
> - poll the<N>th logical address corresponding to a recording device
> * if available, take it
> * if not available continue to the next logical address
> - until we either have a valid address or remain unregistered
I think this is what we are already doing.
Internally, we will iterate over all addresses, but it is not because we
iterate them that we poll.
To illustrate this, let's take the case of a playback device
(device_type 4). The beginning of the loop code:
for (logical_address = 1; logical_address < 15; logical_address++) {
if (logical_address_table[logical_address] != device_type) {
continue;
}
along with the fact that logical_address_table is defined as:
logical_address_table[15] = {0, 1, 1, 3, 4, 5, 3, 3, 4, 1, 3, 4, 2, 2, 0};
means that for logical_addresses 1 to 3, we don't do anything - we just
increment logical_address and continue because the device_type won't
match. Then, for logical_address 4, we have a match on the device_type,
so we poll to see if the address is in use. If it is, then we will skip
over logical_address 5, because the device_type won't match either and
it's only for logical_address 8 that we will poll again.
Thus, I believe that the use of a simple table and the insertion of
continue statements achieves exactly what we need, and is a lot simpler
than the solution your proposed.
On the other hand, I have applied the polling message fix (good call)
and also fixed the file permissions that got changed with the version bump.
Regards,
/Pete
|
|
From: Florian F. <f.f...@gm...> - 2012-01-05 17:14:05
|
The current logical address allocation will iterate over all possible
logical addresses no matter what device type we configured. According
to the specification this is wrong, for instance, if we are a recording
device, we should:
- take the unregistered address
- poll the <N>th logical address corresponding to a recording device
* if available, take it
* if not available continue to the next logical address
- until we either have a valid address or remain unregistered
---
libcec/libcec.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/libcec/libcec.c b/libcec/libcec.c
index 9bcf5c8..18330a9 100644
--- a/libcec/libcec.c
+++ b/libcec/libcec.c
@@ -26,6 +26,7 @@
#include <sys/types.h>
#include "libceci.h"
+#include "decoder.h"
#if defined(LINUX_REALTEK_SOC)
const _ceci_backend* const ceci_backend = &linux_realtek_soc_backend;
@@ -195,12 +196,54 @@ int libcec_set_logical_address(libcec_device_handle* handle, uint8_t logical_add
return ceci_backend->set_logical_address(handle, logical_address);
}
+static uint8_t libcec_get_first_logical_address(uint8_t device_type)
+{
+ switch (device_type & 0x0f) {
+ default:
+ case CEC_DEVTYPE_TV:
+ return 0;
+ case CEC_DEVTYPE_RECORDING:
+ return 1;
+ case CEC_DEVTYPE_TUNER:
+ return 3;
+ case CEC_DEVTYPE_PLAYBACK:
+ return 4;
+ case CEC_DEVTYPE_AUDIO:
+ return 5;
+ }
+}
+
+static uint8_t libcec_get_next_logical_address(uint8_t addr)
+{
+ switch (addr & 0x0f) {
+ default:
+ return 14;
+ case 1:
+ return 2;
+ case 2:
+ return 9;
+ case 3:
+ return 6;
+ case 4:
+ return 8;
+ case 6:
+ return 7;
+ case 7:
+ return 10;
+ case 8:
+ return 11;
+ case 14:
+ case 15:
+ return 15;
+ }
+}
+
DEFAULT_VISIBILITY
int libcec_allocate_logical_address(libcec_device_handle* handle, uint8_t device_type, uint16_t* physical_address)
{
const uint8_t logical_address_table[15] = {0, 1, 1, 3, 4, 5, 3, 3, 4, 1, 3, 4, 2, 2, 0};
int r;
- uint8_t logical_address, polling_message;
+ uint8_t logical_address, polling_message, first_logical_address;
if (handle == NULL) {
return LIBCEC_ERROR_INVALID_PARAM;
@@ -239,7 +282,11 @@ int libcec_allocate_logical_address(libcec_device_handle* handle, uint8_t device
return libcec_set_logical_address(handle, 0);
}
- for (logical_address = 1; logical_address < 15; logical_address++) {
+ first_logical_address = libcec_get_first_logical_address(device_type);
+
+ for (logical_address = first_logical_address;
+ logical_address < 15;
+ logical_address = libcec_get_next_logical_address(logical_address)) {
if (logical_address_table[logical_address] != device_type) {
continue;
}
--
1.7.5.4
|
|
From: Florian F. <f.f...@gm...> - 2012-01-05 17:14:02
|
In order to poll a device, both the initiator and the destination address
must be set to the same value, not broadcast (CEC 10.2.1).
---
libcec/libcec.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libcec/libcec.c b/libcec/libcec.c
index 5375559..9bcf5c8 100644
--- a/libcec/libcec.c
+++ b/libcec/libcec.c
@@ -243,7 +243,7 @@ int libcec_allocate_logical_address(libcec_device_handle* handle, uint8_t device
if (logical_address_table[logical_address] != device_type) {
continue;
}
- polling_message = 0xF0 | logical_address;
+ polling_message = logical_address << 4 | logical_address;
ceci_dbg("querying logical address %d\n", logical_address);
if (libcec_write_message(handle, &polling_message, 1) != LIBCEC_SUCCESS) {
/* error on polling (no ACK) => assume address is free */
--
1.7.5.4
|
|
From: Pete B. <pb...@gm...> - 2012-01-04 18:47:38
|
On 2012.01.03 15:50, Florian Fainelli wrote: >> I'll apply #6 later on today, mostly because I'm trying to remember why >> I was doing it the way I did. > > Allright, I think this needs some clarification anyway. Sorry for the delay. I have now pushed a replacement fix for #6 as I seem to remember that the reason device_type was used is because I didn't have logical address allocation when I coded it, so I used device_type as an easy stopgap. Now that logical address allocation is done, the logical address should be the one used. I also pushed a version bump, since it's been a long time, but that is pretty much only used to add an internal version tag for quick reference - not sure it is actually useful on this project. Regards, /Pete |
|
From: Florian F. <f.f...@gm...> - 2012-01-03 15:50:29
|
On 01/03/12 16:39, Pete Batard wrote: > On 2012.01.03 13:10, Florian Fainelli wrote: >> I just send an updated patch #2 and #5 which were both affected by the >> change. The rest of the patches should apply fine if I am not mistaking. > > I have now applied and pushed #1 to #5 (with a slightly modified #2 as > we can remove the len>= 2 check). > > I'll apply #6 later on today, mostly because I'm trying to remember why > I was doing it the way I did. Allright, I think this needs some clarification anyway. > > Thanks again! You are welcome. > > /Pete > > ------------------------------------------------------------------------------ > Write once. Port to many. > Get the SDK and tools to simplify cross-platform app development. Create > new or port existing apps to sell to consumers worldwide. Explore the > Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join > http://p.sf.net/sfu/intel-appdev > _______________________________________________ > cecd-devel mailing list > cec...@li... > https://lists.sourceforge.net/lists/listinfo/cecd-devel |
|
From: Pete B. <pb...@gm...> - 2012-01-03 15:39:16
|
On 2012.01.03 13:10, Florian Fainelli wrote: > I just send an updated patch #2 and #5 which were both affected by the > change. The rest of the patches should apply fine if I am not mistaking. I have now applied and pushed #1 to #5 (with a slightly modified #2 as we can remove the len >= 2 check). I'll apply #6 later on today, mostly because I'm trying to remember why I was doing it the way I did. Thanks again! /Pete |
|
From: Florian F. <f.f...@gm...> - 2012-01-03 13:10:19
|
Hello Pete, On 01/02/12 18:27, Pete Batard wrote: > Hi Florian, and happy new year to you too! > > Thanks for the patches. I'll try to push 3-6 tonight or tomorrow, after > I have reviewed them. I'll wait for your update on #1 (and possibly #2 > if related?) I just send an updated patch #2 and #5 which were both affected by the change. The rest of the patches should apply fine if I am not mistaking. Thank you. -- Florian > > Regards, > > /Pete > > On 2012.01.02 12:49, Florian Fainelli wrote: >> Hello Pete, >> >> On 12/30/11 15:10, Florian Fainelli wrote: >>> Per specification it is mandatory to answer to an<Abort> message with >>> a<Feature Abort> reply in case the initiator address is not broadcast. >>> Any [Abort Reason] can be used. >> >> First of all, let me wish you a happy new year. This patch is missing >> the opcode parameter in the<Feature Abort> message, will fix that. >> >>> --- >>> cecd/cecd.c | 9 +++++++++ >>> 1 files changed, 9 insertions(+), 0 deletions(-) >>> >>> diff --git a/cecd/cecd.c b/cecd/cecd.c >>> index 67c0a6c..48e62b0 100644 >>> --- a/cecd/cecd.c >>> +++ b/cecd/cecd.c >>> @@ -962,6 +962,15 @@ int main(int argc, char** argv) >>> } >>> len = 0; >>> break; >>> + case CEC_OP_ABORT: >>> + // Only answer to abort if initiator address is not broadcast >>> + if ((buffer[0]& 0x0f) == 0x0f) >>> + break; >>> + buffer[1] = CEC_OP_FEATURE_ABORT; >>> + buffer[2] = CEC_ABORT_REFUSED; >>> + len = 3; >>> + break; >>> + >>> default: >>> // Convert to hash, to match against a conf file command >>> cec_unprocessed[cec_unprocessed_len++] = htab_hash(buffer+1, len-1, >>> htab_cec, 0); > > > ------------------------------------------------------------------------------ > Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex > infrastructure or vast IT resources to deliver seamless, secure access to > virtual desktops. With this all-in-one solution, easily deploy virtual > desktops for less than the cost of PCs and save 60% on VDI infrastructure > costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox > _______________________________________________ > cecd-devel mailing list > cec...@li... > https://lists.sourceforge.net/lists/listinfo/cecd-devel |
|
From: Florian F. <f.f...@gm...> - 2012-01-03 13:09:10
|
In such a case do not answer anything as required by the specification since the message header is invalid. --- Changes since v1: - updated according to patch 2 cecd/cecd.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/cecd/cecd.c b/cecd/cecd.c index f96347b..17c81eb 100644 --- a/cecd/cecd.c +++ b/cecd/cecd.c @@ -978,6 +978,7 @@ int main(int argc, char** argv) buffer[3] = CEC_ABORT_UNRECOGNIZED; break; case LIBCEC_ERROR_INVALID_PARAM: + case LIBCEC_ERROR_OTHER: len = 0; break; default: -- 1.7.5.4 |
|
From: Florian F. <f.f...@gm...> - 2012-01-03 13:08:38
|
In case we do not recognize the opcode, it is mandatory to reply with
a <Feature Abort> message with the proper [Abort Reason]. If the error
returned is INVALID_PARAM, which corresponds to an invalid payload length
or number of operands, do not reply per specification.
---
Changes since v1:
- added check to verify length is big enough to access opcode byte
- removed reply in case of invalid payload lenght as said by specification
- store opcode for insertion in CEC_OP_FEATURE_ABORT reply
cecd/cecd.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/cecd/cecd.c b/cecd/cecd.c
index 48e62b0..f96347b 100644
--- a/cecd/cecd.c
+++ b/cecd/cecd.c
@@ -603,7 +603,7 @@ int main(int argc, char** argv)
uint16_t physical_address = 0xFFFF;
// TODO: check for seq_data overflow
uint16_t seq_data[CEC_MAX_COMMAND_SIZE], seq_len, ucp_unprocessed[CEC_MAX_COMMAND_SIZE], cec_unprocessed[CEC_MAX_COMMAND_SIZE];
- uint8_t i, byte, buffer[CEC_MAX_COMMAND_SIZE];
+ uint8_t i, byte, buffer[CEC_MAX_COMMAND_SIZE], opcode = 0;
uint8_t ucp_unprocessed_len = 0, ucp_processed_len, cec_unprocessed_len = 0, cec_processed_len;
char *target_device, *device_name, *str = NULL, *saveptr = NULL, **key, *val;
@@ -891,7 +891,12 @@ int main(int argc, char** argv)
cecd_log("could not read message (error %d)\n", len);
continue;
}
- libcec_decode_message(buffer, len);
+ r = libcec_decode_message(buffer, len);
+ if (r != LIBCEC_SUCCESS && len >= 2) {
+ opcode = buffer[1];
+ buffer[1] = CEC_OP_ABORT;
+ }
+
if (len <= 1) {
// Ignore ACK, etc.
continue;
@@ -967,8 +972,19 @@ int main(int argc, char** argv)
if ((buffer[0] & 0x0f) == 0x0f)
break;
buffer[1] = CEC_OP_FEATURE_ABORT;
- buffer[2] = CEC_ABORT_REFUSED;
- len = 3;
+ buffer[2] = opcode;
+ switch (r) {
+ case LIBCEC_ERROR_NOT_SUPPORTED:
+ buffer[3] = CEC_ABORT_UNRECOGNIZED;
+ break;
+ case LIBCEC_ERROR_INVALID_PARAM:
+ len = 0;
+ break;
+ default:
+ buffer[3] = CEC_ABORT_REFUSED;
+ break;
+ }
+ len = 4;
break;
default:
@@ -982,6 +998,7 @@ int main(int argc, char** argv)
len = 0;
break;
}
+
if (len) {
if (libcec_write_message(handle, buffer, len)) {
cecd_log("could not send message\n");
--
1.7.5.4
|
|
From: Pete B. <pb...@gm...> - 2012-01-02 17:26:55
|
Hi Florian, and happy new year to you too! Thanks for the patches. I'll try to push 3-6 tonight or tomorrow, after I have reviewed them. I'll wait for your update on #1 (and possibly #2 if related?) Regards, /Pete On 2012.01.02 12:49, Florian Fainelli wrote: > Hello Pete, > > On 12/30/11 15:10, Florian Fainelli wrote: >> Per specification it is mandatory to answer to an<Abort> message with >> a<Feature Abort> reply in case the initiator address is not broadcast. >> Any [Abort Reason] can be used. > > First of all, let me wish you a happy new year. This patch is missing > the opcode parameter in the <Feature Abort> message, will fix that. > >> --- >> cecd/cecd.c | 9 +++++++++ >> 1 files changed, 9 insertions(+), 0 deletions(-) >> >> diff --git a/cecd/cecd.c b/cecd/cecd.c >> index 67c0a6c..48e62b0 100644 >> --- a/cecd/cecd.c >> +++ b/cecd/cecd.c >> @@ -962,6 +962,15 @@ int main(int argc, char** argv) >> } >> len = 0; >> break; >> + case CEC_OP_ABORT: >> + // Only answer to abort if initiator address is not broadcast >> + if ((buffer[0]& 0x0f) == 0x0f) >> + break; >> + buffer[1] = CEC_OP_FEATURE_ABORT; >> + buffer[2] = CEC_ABORT_REFUSED; >> + len = 3; >> + break; >> + >> default: >> // Convert to hash, to match against a conf file command >> cec_unprocessed[cec_unprocessed_len++] = htab_hash(buffer+1, len-1, >> htab_cec, 0); |
|
From: Florian F. <f.f...@gm...> - 2012-01-02 12:49:21
|
Hello Pete, On 12/30/11 15:10, Florian Fainelli wrote: > Per specification it is mandatory to answer to an<Abort> message with > a<Feature Abort> reply in case the initiator address is not broadcast. > Any [Abort Reason] can be used. First of all, let me wish you a happy new year. This patch is missing the opcode parameter in the <Feature Abort> message, will fix that. > --- > cecd/cecd.c | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/cecd/cecd.c b/cecd/cecd.c > index 67c0a6c..48e62b0 100644 > --- a/cecd/cecd.c > +++ b/cecd/cecd.c > @@ -962,6 +962,15 @@ int main(int argc, char** argv) > } > len = 0; > break; > + case CEC_OP_ABORT: > + // Only answer to abort if initiator address is not broadcast > + if ((buffer[0]& 0x0f) == 0x0f) > + break; > + buffer[1] = CEC_OP_FEATURE_ABORT; > + buffer[2] = CEC_ABORT_REFUSED; > + len = 3; > + break; > + > default: > // Convert to hash, to match against a conf file command > cec_unprocessed[cec_unprocessed_len++] = htab_hash(buffer+1, len-1, htab_cec, 0); |
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:52
|
In case the the device type does not match the first available corresponding logical address, we would be forcing the initiator address to the device_type instead of the current logical_address. --- cecd/cecd.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cecd/cecd.c b/cecd/cecd.c index 5e3e110..331edd9 100644 --- a/cecd/cecd.c +++ b/cecd/cecd.c @@ -39,7 +39,7 @@ #include "profile.h" #include "profile_helpers.h" -#define BROADCAST (device_type<<4 | 0x0F) +#define BROADCAST (0x0F) #define MIN(X,Y) ((X) < (Y) ? (X) : (Y)) #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -911,7 +911,7 @@ int main(int argc, char** argv) len = i+2; break; case CEC_OP_GIVE_DEVICE_VENDOR_ID: - buffer[0] = BROADCAST; + buffer[0] |= BROADCAST; buffer[1] = CEC_OP_DEVICE_VENDOR_ID; buffer[2] = (device_oui>>16)&0xFF; buffer[3] = (device_oui>>8)&0xFF; @@ -934,7 +934,7 @@ int main(int argc, char** argv) len = 3; break; case CEC_OP_GIVE_PHYSICAL_ADDRESS: - buffer[0] = BROADCAST; + buffer[0] |= BROADCAST; buffer[1] = CEC_OP_REPORT_PHYSICAL_ADDRESS; buffer[2] = physical_address >> 8; buffer[3] = physical_address & 0xFF; @@ -945,7 +945,7 @@ int main(int argc, char** argv) // Ignore if request is for a different phys_addr if ((buffer[2] != (physical_address >> 8)) || (buffer[3] != (physical_address & 0xFF))) break; - buffer[0] = BROADCAST; + buffer[0] |= BROADCAST; buffer[1] = CEC_OP_ACTIVE_SOURCE; buffer[2] = physical_address >> 8; buffer[3] = physical_address & 0xFF; -- 1.7.5.4 |
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:52
|
In such a case do not answer anything as required by the specification
since the message header is invalid.
---
cecd/cecd.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/cecd/cecd.c b/cecd/cecd.c
index acc9859..5e3e110 100644
--- a/cecd/cecd.c
+++ b/cecd/cecd.c
@@ -973,15 +973,21 @@ int main(int argc, char** argv)
switch (r) {
case LIBCEC_ERROR_NOT_SUPPORTED:
buffer[2] = CEC_ABORT_UNRECOGNIZED;
+ len = 3;
break;
case LIBCEC_ERROR_INVALID_PARAM:
buffer[2] = CEC_ABORT_INVALID_OPERAND;
+ len = 3;
+ break;
+ case LIBCEC_ERROR_OTHER:
+ // Should not answer anything to invalid headers
+ len = 0;
break;
default:
buffer[2] = CEC_ABORT_REFUSED;
+ len = 3;
break;
}
- len = 3;
break;
default:
--
1.7.5.4
|
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:46
|
Per specification it is mandatory to answer to an <Abort> message with a <Feature Abort> reply in case the initiator address is not broadcast. Any [Abort Reason] can be used. --- cecd/cecd.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/cecd/cecd.c b/cecd/cecd.c index 67c0a6c..48e62b0 100644 --- a/cecd/cecd.c +++ b/cecd/cecd.c @@ -962,6 +962,15 @@ int main(int argc, char** argv) } len = 0; break; + case CEC_OP_ABORT: + // Only answer to abort if initiator address is not broadcast + if ((buffer[0] & 0x0f) == 0x0f) + break; + buffer[1] = CEC_OP_FEATURE_ABORT; + buffer[2] = CEC_ABORT_REFUSED; + len = 3; + break; + default: // Convert to hash, to match against a conf file command cec_unprocessed[cec_unprocessed_len++] = htab_hash(buffer+1, len-1, htab_cec, 0); -- 1.7.5.4 |
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:46
|
In case we do not recognize the opcode, or the parameters supplied
are invalid it is mandatory to reply with a <Feature Abort> message
with the proper [Abort Reason].
---
cecd/cecd.c | 18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/cecd/cecd.c b/cecd/cecd.c
index 48e62b0..acc9859 100644
--- a/cecd/cecd.c
+++ b/cecd/cecd.c
@@ -891,7 +891,10 @@ int main(int argc, char** argv)
cecd_log("could not read message (error %d)\n", len);
continue;
}
- libcec_decode_message(buffer, len);
+ r = libcec_decode_message(buffer, len);
+ if (r != LIBCEC_SUCCESS)
+ buffer[1] = CEC_OP_ABORT;
+
if (len <= 1) {
// Ignore ACK, etc.
continue;
@@ -967,7 +970,17 @@ int main(int argc, char** argv)
if ((buffer[0] & 0x0f) == 0x0f)
break;
buffer[1] = CEC_OP_FEATURE_ABORT;
- buffer[2] = CEC_ABORT_REFUSED;
+ switch (r) {
+ case LIBCEC_ERROR_NOT_SUPPORTED:
+ buffer[2] = CEC_ABORT_UNRECOGNIZED;
+ break;
+ case LIBCEC_ERROR_INVALID_PARAM:
+ buffer[2] = CEC_ABORT_INVALID_OPERAND;
+ break;
+ default:
+ buffer[2] = CEC_ABORT_REFUSED;
+ break;
+ }
len = 3;
break;
@@ -982,6 +995,7 @@ int main(int argc, char** argv)
len = 0;
break;
}
+
if (len) {
if (libcec_write_message(handle, buffer, len)) {
cecd_log("could not send message\n");
--
1.7.5.4
|
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:45
|
Once we made sure the message length is valid, compute src and dst address
once and do not do that twice.
---
libcec/decoder.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/libcec/decoder.c b/libcec/decoder.c
index 8fd7655..37764c2 100644
--- a/libcec/decoder.c
+++ b/libcec/decoder.c
@@ -196,9 +196,12 @@ int libcec_decode_message(uint8_t* message, size_t length)
return LIBCEC_ERROR_INVALID_PARAM;
}
+ src = message[0] >> 4;
+ dst = message[0] & 0x0F;
+
// Polling Message
if (length == 1) {
- ceci_info(" o %1X->%1X: <Polling Message>", src = message[0] >> 4, dst = message[0] & 0x0F);
+ ceci_info(" o %1X->%1X: <Polling Message>", src, dst);
return LIBCEC_SUCCESS;
}
@@ -206,12 +209,13 @@ int libcec_decode_message(uint8_t* message, size_t length)
ceci_warn("unsupported Opcode: %02X", message[1]);
return LIBCEC_ERROR_NOT_SUPPORTED;
}
+
if ( (length-2 < msg_min_max[msg_props[message[1]]&0x1F][0])
|| (length-2 > msg_min_max[msg_props[message[1]]&0x1F][1]) ) {
ceci_warn("invalid payload length for opcode: %02X", message[1]);
return LIBCEC_ERROR_INVALID_PARAM;
}
- ceci_info(" o %1X->%1X: <%s>", src = message[0] >> 4, dst = message[0] & 0x0F,
+ ceci_info(" o %1X->%1X: <%s>", src, dst,
msg_description[msg_index[message[1]]]);
display_buffer_hex(message+1, length-1);
--
1.7.5.4
|
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:45
|
HDMI CEC Compliance Testing Specification imposes a device not to answer to:
- messages received as broadcast while they are supposed to be directed
- messages received as directed with a broadcast address
in such a case return LIBCEC_ERROR_OTHER
---
libcec/decoder.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/libcec/decoder.c b/libcec/decoder.c
index 37764c2..2181310 100644
--- a/libcec/decoder.c
+++ b/libcec/decoder.c
@@ -210,6 +210,17 @@ int libcec_decode_message(uint8_t* message, size_t length)
return LIBCEC_ERROR_NOT_SUPPORTED;
}
+ // Broadcasted messages received as directed messages
+ if ((dst == 0x0F) && ((msg_props[message[1]] & 0x40) == 0)) {
+ ceci_warn("broadcast message received as directed: %02X", message[1]);
+ return LIBCEC_ERROR_OTHER;
+ }
+
+ if ((dst != 0x0F) && ((msg_props[message[1]] & 0x20) == 0)) {
+ ceci_warn("directed message received as broadcast: %02X", message[1]);
+ return LIBCEC_ERROR_OTHER;
+ }
+
if ( (length-2 < msg_min_max[msg_props[message[1]]&0x1F][0])
|| (length-2 > msg_min_max[msg_props[message[1]]&0x1F][1]) ) {
ceci_warn("invalid payload length for opcode: %02X", message[1]);
--
1.7.5.4
|
|
From: Florian F. <f.f...@gm...> - 2011-12-30 14:11:45
|
All of these patches are related to the compliance testing I performed using both our own implementation and cecd. They should make CEC devices conform better to the HDMI CEC CTS 1.3. Florian Fainelli (6): [cecd] answer to <Abort> opcode [cecd] check and handle libcec_decode_message return values [libcec] compute src and dst once and for all [libcec] check unicast/broadcast flag with destination address [cecd] handle LIBCEC_ERROR_OTHER from libcec_decode_message [cecd] fix BROADCAST usage cecd/cecd.c | 39 ++++++++++++++++++++++++++++++++++----- libcec/decoder.c | 19 +++++++++++++++++-- 2 files changed, 51 insertions(+), 7 deletions(-) -- 1.7.5.4 |
|
From: Pete B. <pb...@gm...> - 2011-12-22 15:34:21
|
Both of the previous patches make sense. They have now been committed. Thanks! /Pete |
|
From: Florian F. <f.f...@gm...> - 2011-12-21 14:41:30
|
---
libcec/decoder.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/libcec/decoder.h b/libcec/decoder.h
index cced415..feaa051 100644
--- a/libcec/decoder.h
+++ b/libcec/decoder.h
@@ -1,3 +1,6 @@
+#ifndef __LIBCEC_DECODER_H__
+#define __LIBCEC_DECODER_H__
+
/*
* decoder - CEC message decoding
*
@@ -568,3 +571,5 @@ typedef struct {
#define CEC_UI_F4_OR_YELLOW 0x74
#define CEC_UI_F5 0x75
#define CEC_UI_DATA 0x76
+
+#endif
--
1.7.5.4
|
2 messages has been excluded from this view by a project administrator.
